loggerhead

Merge lp:~pauljnixon/loggerhead/context_in_diffs into lp:loggerhead

  1. context_in_diffs
  2. Merge into trunk-rich
Proposed by Paul Nixon
Status: Superseded
Proposed branch: lp:~pauljnixon/loggerhead/context_in_diffs
Merge into: lp:loggerhead
Diff against target: 180 lines (+54/-14)
4 files modified
loggerhead/controllers/diff_ui.py (+19/-6)
loggerhead/controllers/filediff_ui.py (+8/-3)
loggerhead/static/javascript/diff.js (+24/-3)
loggerhead/templates/revision.pt (+3/-2)
To merge this branch: bzr merge lp:~pauljnixon/loggerhead/context_in_diffs
Reviewer Review Type Date Requested Status
Loggerhead Reviewers Pending
Review via email: mp+117102@code.launchpad.net

This proposal has been superseded by a proposal from 2012-08-08.

Description of the change

Adds code to the "revision" page to configure the lines of context
shown in the diffs on that page.

Dependent on lp:~pauljnixon/bzr/context_in_diffs
(merged into lp:bzr as of July 28).

To post a comment you must log in.
Revision history for this message
Benji York (benji) wrote :

Am I reading the new code correctly in that any pre-existing links (in
the previous format) will generate different results than they did
before this change? If so, that seems unfortunate.

How about making the number of lines of context an optional parameter
instead, like so:

    /diff/<rev_id>/<rev_id>?context=<context_lines>

This arrangement seems to better reflect the semantics of the situation
as well as both allowing old links continue to work and permit the
number of lines of context to be optional.

Revision history for this message
Paul Nixon (pauljnixon) wrote :

Benji:

That's a good idea. I'll work on that.

Thanks,
---Paul

On Mon, Jul 30, 2012 at 1:28 PM, Benji York <email address hidden> wrote:
> Am I reading the new code correctly in that any pre-existing links (in
> the previous format) will generate different results than they did
> before this change? If so, that seems unfortunate.
>
> How about making the number of lines of context an optional parameter
> instead, like so:
>
> /diff/<rev_id>/<rev_id>?context=<context_lines>
>
> This arrangement seems to better reflect the semantics of the situation
> as well as both allowing old links continue to work and permit the
> number of lines of context to be optional.
>
> --
> https://code.launchpad.net/~pauljnixon/loggerhead/context_in_diffs/+merge/117102
> You are the owner of lp:~pauljnixon/loggerhead/context_in_diffs.

Revision history for this message
Richard Harding (rharding) wrote :

Switched back to WIP since there's tweaking going on. Please change back to needs review once adjusted.

Revision history for this message
Paul Nixon (pauljnixon) wrote :

Switched back to needs review.

> Switched back to WIP since there's tweaking going on. Please change back to
> needs review once adjusted.

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'loggerhead/controllers/diff_ui.py'
2--- loggerhead/controllers/diff_ui.py 2012-02-08 01:50:02 +0000
3+++ loggerhead/controllers/diff_ui.py 2012-07-31 20:28:34 +0000
4@@ -19,7 +19,7 @@
5 from cStringIO import StringIO
6 import time
7
8-from paste.request import path_info_pop
9+from paste.request import path_info_pop, parse_querystring
10
11 from bzrlib.diff import show_diff_trees
12 from bzrlib.revision import NULL_REVISION
13@@ -31,7 +31,10 @@
14 """Class to output a diff for a single file or revisions."""
15
16 def __call__(self, environ, start_response):
17- # /diff/<rev_id>/<rev_id>
18+ # End of URL is now /diff/<rev_id>?context=<context_lines>
19+ # or /diff/<rev_id>/<rev_id>?context=<context_lines>
20+ # This allows users to choose how much context they want to see.
21+ # Old format was /diff/<rev_id>/<rev_id> or /diff/<rev_id>
22 """Default method called from /diff URL."""
23 z = time.time()
24
25@@ -42,8 +45,18 @@
26 break
27 args.append(arg)
28
29+ numlines = 3 # This is the default.
30+
31+ opts = parse_querystring(environ)
32+ for opt in opts:
33+ if opt[0] == 'context':
34+ try:
35+ numlines = int(opt[1])
36+ except ValueError:
37+ pass
38+
39 revid_from = args[0]
40- # Convert a revno to a revid if we get a revno
41+ # Convert a revno to a revid if we get a revno.
42 revid_from = self._history.fix_revid(revid_from)
43 change = self._history.get_changes([revid_from])[0]
44
45@@ -60,12 +73,12 @@
46
47 diff_content_stream = StringIO()
48 show_diff_trees(revtree1, revtree2, diff_content_stream,
49- old_label='', new_label='')
50+ old_label='', new_label='', context=numlines)
51
52 content = diff_content_stream.getvalue()
53
54- self.log.info('/diff %r:%r in %r secs' % (revid_from, revid_to,
55- time.time() - z))
56+ self.log.info('/diff %r:%r in %r secs with %r context' % (revid_from, revid_to,
57+ time.time() - z, numlines))
58
59 revno1 = self._history.get_revno(revid_from)
60 revno2 = self._history.get_revno(revid_to)
61
62=== modified file 'loggerhead/controllers/filediff_ui.py'
63--- loggerhead/controllers/filediff_ui.py 2011-06-27 15:05:06 +0000
64+++ loggerhead/controllers/filediff_ui.py 2012-07-31 20:28:34 +0000
65@@ -53,7 +53,7 @@
66 return chunks
67
68
69-def diff_chunks_for_file(repository, file_id, compare_revid, revid):
70+def diff_chunks_for_file(repository, file_id, compare_revid, revid, context_lines):
71 lines = {}
72 args = []
73 for r in (compare_revid, revid):
74@@ -65,7 +65,7 @@
75 lines[r] = osutils.split_lines(''.join(bytes_iter))
76 buffer = StringIO()
77 try:
78- diff.internal_diff('', lines[compare_revid], '', lines[revid], buffer)
79+ diff.internal_diff('', lines[compare_revid], '', lines[revid], buffer, context_lines=context_lines)
80 except errors.BinaryFile:
81 difftext = ''
82 else:
83@@ -84,8 +84,13 @@
84 compare_revid = urllib.unquote(self.args[1])
85 file_id = urllib.unquote(self.args[2])
86
87+ try:
88+ context_lines = int(kwargs['context'])
89+ except (KeyError, ValueError):
90+ context_lines = 3 # This is the default.
91+
92 chunks = diff_chunks_for_file(
93- self._history._branch.repository, file_id, compare_revid, revid)
94+ self._history._branch.repository, file_id, compare_revid, revid, context_lines)
95
96 return {
97 'chunks': chunks,
98
99=== modified file 'loggerhead/static/javascript/diff.js'
100--- loggerhead/static/javascript/diff.js 2012-02-22 08:43:19 +0000
101+++ loggerhead/static/javascript/diff.js 2012-07-31 20:28:34 +0000
102@@ -159,6 +159,26 @@
103 }
104 }
105
106+var original_diff_download_link = null;
107+
108+function compute_diff_links() {
109+ var diffs = Y.all('.diff');
110+ if (diffs == null) return;
111+ var numlines = document.getElementById('contextLines').value;
112+ diffs.each(
113+ function(item, i)
114+ {
115+ item.collapsable.source = global_path + '+filediff/' + link_data[item.get('id')] + '?context=' + numlines;
116+ });
117+ if(original_diff_download_link == null) original_diff_download_link = document.getElementById('download_link').href;
118+ document.getElementById('download_link').href = original_diff_download_link + '?context=' + numlines;
119+}
120+
121+function get_num_lines() {
122+ var numlines = document.getElementById('contextLines').value;
123+ return numlines;
124+}
125+
126 Y.on(
127 "domready", function () {
128 Y.all(".show_if_js").removeClass("show_if_js");
129@@ -174,16 +194,16 @@
130 }
131 var diffs = Y.all('.diff');
132 if (diffs == null) return;
133+ var numlines = document.getElementById('contextLines').value;
134 diffs.each(
135 function(item, i)
136 {
137- var source_url = null;
138- if (!specific_path)
139- source_url = global_path + '+filediff/' + link_data[item.get('id')];
140+ var source_url = global_path + '+filediff/' + link_data[item.get('id')] + '?context=' + numlines;
141 item.query('.the-link').on(
142 'click',
143 function(e) {
144 e.preventDefault();
145+ item.collapsable.source = global_path + '+filediff/' + link_data[item.get('id')] + '?context=' + document.getElementById('contextLines').value;
146 collapsable.toggle();
147 });
148 var collapsable = new Collapsable(
149@@ -198,6 +218,7 @@
150 });
151 item.collapsable=collapsable;
152 });
153+ compute_diff_links();
154 if (window.location.hash && !specific_path) {
155 zoom_to_diff(window.location.hash.substring(1));
156 }
157
158=== modified file 'loggerhead/templates/revision.pt'
159--- loggerhead/templates/revision.pt 2011-11-23 08:33:55 +0000
160+++ loggerhead/templates/revision.pt 2012-07-31 20:28:34 +0000
161@@ -80,9 +80,9 @@
162 </a>
163 </li>
164 <li>
165- <a tal:condition="python:compare_revid is None"
166+ <a id="download_link" tal:condition="python:compare_revid is None"
167 tal:attributes="href python:url(['/diff', change.revno], clear=1)">download diff</a>
168- <a tal:condition="python:compare_revid is not None"
169+ <a id="download_link" tal:condition="python:compare_revid is not None"
170 tal:attributes="href python:url(['/diff', change.revno, history.get_revno(compare_revid)], clear=1)">download diff</a>
171 </li>
172 <li tal:condition="python:can_export">
173@@ -123,6 +123,7 @@
174 <p class="expand show_if_js"><a id="toggle_unified_sbs" href="#">Show diffs side-by-side</a></p>
175 <p class="codin"><img tal:attributes="src python:branch.static_url('/static/images/newCode.gif')" alt="added" /> added</p>
176 <p class="codin"><img tal:attributes="src python:branch.static_url('/static/images/deleteCode.gif')" alt="removed" /> removed</p>
177+ <form onSubmit="compute_diff_links(); return false">Lines of Context:<input type="text" id="contextLines" size="2" length="7" value="5" onKeyUp="compute_diff_links(); return false"/></form>
178 <div class="clear"><!-- --></div>
179
180 </tal:block>

Subscribers

People subscribed via source and target branches