[PATCH 3 of 3 V2] revset: add a changes(file, fromline, toline[, rev]) revset

Denis Laxalde denis at laxalde.org
Wed Dec 28 16:46:28 EST 2016


FUJIWARA Katsunori a écrit :
> At Mon, 26 Dec 2016 22:47:14 +0900,
> Yuya Nishihara wrote:
>>
>> On Sun, 25 Dec 2016 18:06:58 -0500, Augie Fackler wrote:
>>>> On Dec 25, 2016, at 4:44 AM, Yuya Nishihara <yuya at tcha.org> wrote:
>>>> On Sat, 24 Dec 2016 14:56:51 -0500, Augie Fackler wrote:
>>>>> On Fri, Dec 16, 2016 at 02:30:15PM +0100, Denis Laxalde wrote:
>>>>>> # HG changeset patch
>>>>>> # User Denis Laxalde <denis.laxalde at logilab.fr>
>>>>>> # Date 1480086890 -3600
>>>>>> #      Fri Nov 25 16:14:50 2016 +0100
>>>>>> # Node ID 5e5ec2ade2cfc829cffba145193da5801c5b20e7
>>>>>> # Parent  c8dfd10c5865cfe882a00595743f3f709f41317f
>>>>>> # EXP-Topic linerange-log/revset
>>>>>> revset: add a changes(file, fromline, toline[, rev]) revset
>>>>>
>>>>> I'm +1 on the series as a whole. I'll do one more pass over the diff
>>>>> stuff to try and understand it, but realistically I'm too far removed
>>>>> from that code right now to give it a proper review.
>>>>
>>>> Looks generally good to me per comparing with the V1 patches.
>
> I caught "abort: line range exceeds file size" failure by
> "changes(foo, 1, 2)" at the end of test script below. Is this failure
> expected result ?

There appears to be a missing case in mdiff.blocksinrange(). Actually,
it was there in my V1 implementation, but was dropped because I could
not figure out what it was for. Now I got a test case, thanks!

>   $ hg init repo
>   $ cd repo
>
>   $ cat > foo <<EOF
>   > 05 at #2, 03 at #1, 01 at #0
>   > 06,at #2, 04 at #1, 02 at #0
>   > EOF
>   $ hg commit -Am '#0'
>   adding foo
>
>   $ cat > foo <<EOF
>   > 03 at #2, 01 at #1
>   > 04 at #2, 02 at #1
>   > 05 at #2, 03 at #1, 01 at #0
>   > 06,at #2, 04 at #1, 02 at #0
>   > EOF
>   $ hg commit -m '#1'
>
>   $ cat > foo <<EOF
>   > 01 at #2
>   > 02 at #2
>   > 03 at #2, 01 at #1
>   > 04 at #2, 02 at #1
>   > 05 at #2, 03 at #1, 01 at #0
>   > 06,at #2, 04 at #1, 02 at #0
>   > EOF
>   $ hg commit -m '#2'
>
>   $ hg annotate foo
>   2: 01 at #2
>   2: 02 at #2
>   1: 03 at #2, 01 at #1
>   1: 04 at #2, 02 at #1
>   0: 05 at #2, 03 at #1, 01 at #0
>   0: 06,at #2, 04 at #1, 02 at #0
>
> If we can expect "changes(foo, 1, 2)" to run successfully (at least, I
> want :-)), which does it return ?
>
>   - 0, 1, 2
>     because line range 1 - 2 is changed at these revisions
>
>   - 2
>     because contents of line range 1 -2 at tip appear only at rev 2


The latter, I'd say. Tweaking back to my previous implementation, I get
0, 2 but 0 is wrong because of a wrong termination condition in
blockancestors function. I wasn't sure about this and your test case
really helps here too.

> According to description "Line range corresponds to 'file' content at
> 'rev'" in this patch, the latter seems suitable.
>
> But on the other hand, "chages(foo, 3, 4)" returns 0 and 1 in the case
> above. This works like the former.

This one should return 1 only, 0 is wrong for the same reason as above.

I'll try to have a V3 fixing these issues by next week.


More information about the Mercurial-devel mailing list