[PATCH] hgweb: fix diff hunks filtering by line range in webutil.diffs()

Gregory Szorc gregory.szorc at gmail.com
Thu Mar 30 10:30:47 EDT 2017



> On Mar 30, 2017, at 01:27, Denis Laxalde <denis at laxalde.org> wrote:
> 
> Gregory Szorc a écrit :
>>> On Wed, Mar 29, 2017 at 6:10 AM, Denis Laxalde <denis at laxalde.org> wrote:
>>> 
>>> # HG changeset patch
>>> # User Denis Laxalde <denis.laxalde at logilab.fr>
>>> # Date 1490782027 -7200
>>> #      Wed Mar 29 12:07:07 2017 +0200
>>> # Node ID 28297d6c3ae842f4c26f878bf5b107619366fbd5
>>> # Parent  e540846c67e0f838bcdb1db567a57df28d92491c
>>> # Available At http://hg.logilab.org/users/dlaxalde/hg
>>> #              hg pull http://hg.logilab.org/users/dlaxalde/hg -r
>>> 28297d6c3ae8
>>> # EXP-Topic linerange-log/hgweb-filelog
>>> hgweb: fix diff hunks filtering by line range in webutil.diffs()
>>> 
>>> The previous clause for filter out a diff hunk was too restrictive. We
>>> need to
>>> consider the following cases (assuming linerange=(lb, ub) and the @s2,l2
>>> hunkrange):
>>> 
>>>            <-(s2)--------(s2+l2)->
>>>      <-(lb)---(ub)->
>>>               <-(lb)---(ub)->
>>>                           <-(lb)---(ub)->
>>> 
>>> previously on the first and last situations were considered.
>>> 
>>> In test-hgweb-filelog.t, add a couple of lines at the beginning of file
>>> "b" so
>>> that the line range we will follow does not start at the beginning of file.
>>> This covers the change in aforementioned diff hunk filter clause.
>>> 
>> I think this is better. But there's still some oddity here. Specifically, a
>> number of diffs render extra lines that don't appear to be relevant to the
>> initial line range. For example, on mozilla-central, the request for
>> /log/60d7a0496a36/layout/base/nsCSSFrameConstructor.cpp?patch=&linerange=519:534
>> has an initial diff covering 13,000+ lines (the entire file). It seems we
>> went from showing too little diff (no diff) to too much diff (all diff).
> 
> 
> As far as I can tell, in all diffs on this page, there are only hunks
> (in general one, sometimes two) in which there are changes that concern
> the initial line range (GetLastIBSplitSibling function, renamed as
> GetLastSpecialSibling). Do you agree with that?
> 
> It is true that some hunks spread far from the target line range (for
> instance the second row "Bug 508665 - part 3 [...]" (75c14b62556e) or
> "Fix crash bug 393517. [...]" (8d561edb0012)). This is because the
> algorithm does not split hunks but only filter them. In other words, if
> mdiff.allblocks() (or more likely bdiff.blocks()) produces "too big"
> hunks, there's nothing we can do with the current implementation.

Ahh, I understand now. The new behavior obviously makes sense given the underlying implementation of being hunk based instead of line based.


More information about the Mercurial-devel mailing list