loggerhead

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

context_in_diffs
Merge into trunk-rich

Proposed by Paul Nixon on 2012-08-08

Status:	Merged
Merged at revision:	482
Proposed branch:	lp://staging/~pauljnixon/loggerhead/context_in_diffs
Merge into:	lp://staging/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://staging/~pauljnixon/loggerhead/context_in_diffs
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Benji York (community)	code	2012-08-08	Approve on 2012-08-13
Review via email: mp+118761@code.staging.launchpad.net

This proposal supersedes a proposal from 2012-07-27.

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).

This has been changed to be backwards-compatible with old URLs.

Revision history for this message

Benji York (benji) wrote on 2012-07-30: Posted in a previous version of this proposal

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 on 2012-07-30: Posted in a previous version of this proposal

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.