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

FUJIWARA Katsunori foozy at lares.dti.ne.jp
Tue Dec 27 01:10:20 EST 2016


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 ?

  $ 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

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.

Sorry, if I overlooked the conclusion about this point in V1 post,
"history at diff blocks level" thead, and/or 4.0 sprint.


> > >>> --- a/mercurial/revset.py
> > >>> +++ b/mercurial/revset.py
> > >>> @@ -1350,6 +1350,42 @@ def modifies(repo, subset, x):
> > >>>     pat = getstring(x, _("modifies requires a pattern"))
> > >>>     return checkstatus(repo, subset, pat, 0)
> > >>> 
> > >>> + at predicate('changes(file, fromline, toline, rev)', safe=True)
> > >>> +def changes(repo, subset, x):
> > >>> +    """Changesets modifying `file` in line range ('fromline', 'toline').
> > >>> +
> > >>> +    Line range corresponds to 'file' content at 'rev'. If rev is not
> > >>> +    specified, working directory's parent is used.
> > >>> +    """
> > > 
> > > My only concern is the name and signature of this function:
> > > 
> > > - "changes" seems unclear (but I have no better idea.)
> > 
> > I also was wishing for a better name, but couldn’t come up with one.
> > 
> > > - what if we want to support multiple fromline:toline pairs?
> >
> > Hm. Could that just be done as `changes(file, 10, 20, tip) or changes(file, 40, 50, tip)`? That seems like a good spot.
>
> I have some ideas, but none of them look nice.
> 
>   changes(file, 10, 20, tip, 30, 40)      # changes(file, from, to[, rev, from, to, ...])
>   changes(file, '10:20,30:40', tip)       # changes(file, from_to_str[, rev])
>   changes(file, 10, 20, 30, 40, rev=tip)  # changes(file, from, to, ...[, rev=rev])
>   changes(file, 10:20, 30:40, rev=tip)    # changes(file, from:to, ...[, rev=rev])
>   changes(file, tip, 10, 20, 30, 40)      # changes(file, rev, from, to, ...)
>   changes(file, tip, 10:20, 30:40)        # changes(file, rev, from:to, ...)

I like from:to style (#4 or #6 above), because of similarity to Python
indexing style, but current revset parsing seems to treat this format
as "range expression" forcibly :-<

But quoting like #2 causes inconvenience.

Therefore, #3 or #5 above seems reasonable in this list, for me.

BTW, to specify arbitrary ranges without knowledge of actual file
contents (e.g. at automation by scripting), how about these ?

  - allow omitting "toline", for "to the last line"
  - allow negative indexing, for "from/to last N lines"


> > > Since our blockancestors() still has a limitation on merges, I think it's
> > > good to start this as an experimental revset function.
> > 
> > Ah, good point. Do we have other experimental revsets as prior art for how to handle that? If not, did you have something in mind?
> 
> wdir() is still experimental, but which might not be a good example.
> Experimental and internal revsets are just undocumented.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

-- 
----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy at lares.dti.ne.jp


More information about the Mercurial-devel mailing list