Patchwork [4,of,4,hyperblame] annotate: add a new experimental --skip option to skip revs

login
register
mail settings
Submitter Siddharth Agarwal
Date May 25, 2017, 2:39 a.m.
Message ID <6edfa6f240f311e7aa5f0002c991e86a-964d56b0@7511894063d3764ff01ea8111f5a004d7dd700ed078797c204a24e620ddb965c>
Download mbox | patch
Permalink /patch/20891/
State Accepted
Headers show

Comments

Siddharth Agarwal - May 25, 2017, 2:39 a.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1495679973 25200
#      Wed May 24 19:39:33 2017 -0700
# Node ID 0c6f6c4b027191c30d867fb4eb0005682ea90a76
# Parent  134dcc1222e4b992b4a60c44c9b9ed8d10422632
annotate: add a new experimental --skip option to skip revs

This option is most useful for mechanical code modifications, especially ones
that retain the same number of lines.
Siddharth Agarwal - May 25, 2017, 11:49 p.m.
On 5/24/17 7:39 PM, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1495679973 25200
> #      Wed May 24 19:39:33 2017 -0700
> # Node ID 0c6f6c4b027191c30d867fb4eb0005682ea90a76
> # Parent  134dcc1222e4b992b4a60c44c9b9ed8d10422632
> annotate: add a new experimental --skip option to skip revs
>
> This option is most useful for mechanical code modifications, especially ones
> that retain the same number of lines.

BTW, some questions that would be worth addressing at some point:

1. Should we call it --skip, --ignore or something else? I'm worried 
that "ignore" is too close to hgignore, which is completely unrelated.
2. For this to be effective, we'd probably want to have a checked in 
file that can apply a default list of revsets to ignore. What should 
that file be called, and what should be the UI around that look like? 
Will we want to enable this by default with hg blame, and then have a 
way to disable it? If additional revsets are specified with 
--skip/--ignore, how do those interact?
3. git hyper-blame adds a * at the end of each commit that god skipped. 
Do we want to do anything similar? I'm worried that if we turn something 
like that on by default, we'll break tools that parse annotate output.

- Siddharth

>
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -262,7 +262,8 @@ def addremove(ui, repo, *pats, **opts):
>       ('d', 'date', None, _('list the date (short with -q)')),
>       ('n', 'number', None, _('list the revision number (default)')),
>       ('c', 'changeset', None, _('list the changeset')),
> -    ('l', 'line-number', None, _('show line number at the first appearance'))
> +    ('l', 'line-number', None, _('show line number at the first appearance')),
> +    ('', 'skip', [], _('revision to not display (EXPERIMENTAL)'), _('REV')),
>       ] + diffwsopts + walkopts + formatteropts,
>       _('[-r REV] [-f] [-a] [-u] [-d] [-n] [-c] [-l] FILE...'),
>       inferrepo=True)
> @@ -368,6 +369,10 @@ def annotate(ui, repo, *pats, **opts):
>       follow = not opts.get('no_follow')
>       diffopts = patch.difffeatureopts(ui, opts, section='annotate',
>                                        whitespace=True)
> +    skiprevs = opts.get('skip')
> +    if skiprevs:
> +        skiprevs = scmutil.revrange(repo, skiprevs)
> +
>       for abs in ctx.walk(m):
>           fctx = ctx[abs]
>           if not opts.get('text') and fctx.isbinary():
> @@ -375,7 +380,7 @@ def annotate(ui, repo, *pats, **opts):
>               continue
>   
>           lines = fctx.annotate(follow=follow, linenumber=linenumber,
> -                              diffopts=diffopts)
> +                              skiprevs=skiprevs, diffopts=diffopts)
>           if not lines:
>               continue
>           formats = []
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -949,7 +949,8 @@ class basefilectx(object):
>               return p[1]
>           return filectx(self._repo, self._path, fileid=-1, filelog=self._filelog)
>   
> -    def annotate(self, follow=False, linenumber=False, diffopts=None):
> +    def annotate(self, follow=False, linenumber=False, skiprevs=None,
> +                 diffopts=None):
>           '''returns a list of tuples of ((ctx, number), line) for each line
>           in the file, where ctx is the filectx of the node where
>           that line was last changed; if linenumber parameter is true, number is
> @@ -1044,7 +1045,10 @@ class basefilectx(object):
>               if ready:
>                   visit.pop()
>                   curr = decorate(f.data(), f)
> -                curr = _annotatepair([hist[p] for p in pl], f, curr, False,
> +                skipchild = False
> +                if skiprevs is not None:
> +                    skipchild = f._changeid in skiprevs
> +                curr = _annotatepair([hist[p] for p in pl], f, curr, skipchild,
>                                        diffopts)
>                   for p in pl:
>                       if needed[p] == 1:
> diff --git a/tests/test-annotate.t b/tests/test-annotate.t
> --- a/tests/test-annotate.t
> +++ b/tests/test-annotate.t
> @@ -217,6 +217,77 @@ annotate after rename merge with -l
>     3 b:5: b5
>     7 b:7: d
>   
> +--skip nothing (should be the same as no --skip at all)
> +
> +  $ hg annotate -nlf b --skip '1::0'
> +  0 a:1: a
> +  6 b:2: z
> +  1 a:3: a
> +  3 b:4: b4
> +  4 b:5: c
> +  3 b:5: b5
> +  7 b:7: d
> +
> +--skip a modified line
> +
> +  $ hg annotate -nlf b --skip 6
> +  0 a:1: a
> +  1 a:2: z
> +  1 a:3: a
> +  3 b:4: b4
> +  4 b:5: c
> +  3 b:5: b5
> +  7 b:7: d
> +
> +--skip added lines (and test multiple skip)
> +
> +  $ hg annotate -nlf b --skip 3
> +  0 a:1: a
> +  6 b:2: z
> +  1 a:3: a
> +  1 a:3: b4
> +  4 b:5: c
> +  1 a:3: b5
> +  7 b:7: d
> +
> +  $ hg annotate -nlf b --skip 4
> +  0 a:1: a
> +  6 b:2: z
> +  1 a:3: a
> +  3 b:4: b4
> +  1 a:3: c
> +  3 b:5: b5
> +  7 b:7: d
> +
> +  $ hg annotate -nlf b --skip 3 --skip 4
> +  0 a:1: a
> +  6 b:2: z
> +  1 a:3: a
> +  1 a:3: b4
> +  1 a:3: c
> +  1 a:3: b5
> +  7 b:7: d
> +
> +  $ hg annotate -nlf b --skip 'merge()'
> +  0 a:1: a
> +  6 b:2: z
> +  1 a:3: a
> +  3 b:4: b4
> +  4 b:5: c
> +  3 b:5: b5
> +  3 b:5: d
> +
> +--skip everything -- use the revision the file was introduced in
> +
> +  $ hg annotate -nlf b --skip 'all()'
> +  0 a:1: a
> +  0 a:1: z
> +  0 a:1: a
> +  0 a:1: b4
> +  0 a:1: c
> +  0 a:1: b5
> +  0 a:1: d
> +
>   Issue2807: alignment of line numbers with -l
>   
>     $ echo more >> b
> diff --git a/tests/test-completion.t b/tests/test-completion.t
> --- a/tests/test-completion.t
> +++ b/tests/test-completion.t
> @@ -217,7 +217,7 @@ Show an error if we use --options with a
>   Show all commands + options
>     $ hg debugcommands
>     add: include, exclude, subrepos, dry-run
> -  annotate: rev, follow, no-follow, text, user, file, date, number, changeset, line-number, ignore-all-space, ignore-space-change, ignore-blank-lines, include, exclude, template
> +  annotate: rev, follow, no-follow, text, user, file, date, number, changeset, line-number, skip, ignore-all-space, ignore-space-change, ignore-blank-lines, include, exclude, template
>     clone: noupdate, updaterev, rev, branch, pull, uncompressed, ssh, remotecmd, insecure
>     commit: addremove, close-branch, amend, secret, edit, interactive, include, exclude, message, logfile, date, user, subrepos
>     diff: rev, change, text, git, binary, nodates, noprefix, show-function, reverse, ignore-all-space, ignore-space-change, ignore-blank-lines, unified, stat, root, include, exclude, subrepos
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Phillip Cohen - May 26, 2017, 1:28 a.m.
> 1. Should we call it --skip, --ignore or something else?

"--skip" feels more intuitive to me, but IANANE.

On Thu, May 25, 2017 at 4:49 PM, Siddharth Agarwal <sid@less-broken.com> wrote:
> On 5/24/17 7:39 PM, Siddharth Agarwal wrote:
>>
>> # HG changeset patch
>> # User Siddharth Agarwal <sid0@fb.com>
>> # Date 1495679973 25200
>> #      Wed May 24 19:39:33 2017 -0700
>> # Node ID 0c6f6c4b027191c30d867fb4eb0005682ea90a76
>> # Parent  134dcc1222e4b992b4a60c44c9b9ed8d10422632
>> annotate: add a new experimental --skip option to skip revs
>>
>> This option is most useful for mechanical code modifications, especially
>> ones
>> that retain the same number of lines.
>
>
> BTW, some questions that would be worth addressing at some point:
>
> 1. Should we call it --skip, --ignore or something else? I'm worried that
> "ignore" is too close to hgignore, which is completely unrelated.
> 2. For this to be effective, we'd probably want to have a checked in file
> that can apply a default list of revsets to ignore. What should that file be
> called, and what should be the UI around that look like? Will we want to
> enable this by default with hg blame, and then have a way to disable it? If
> additional revsets are specified with --skip/--ignore, how do those
> interact?
> 3. git hyper-blame adds a * at the end of each commit that god skipped. Do
> we want to do anything similar? I'm worried that if we turn something like
> that on by default, we'll break tools that parse annotate output.
>
> - Siddharth
>
>
>>
>> diff --git a/mercurial/commands.py b/mercurial/commands.py
>> --- a/mercurial/commands.py
>> +++ b/mercurial/commands.py
>> @@ -262,7 +262,8 @@ def addremove(ui, repo, *pats, **opts):
>>       ('d', 'date', None, _('list the date (short with -q)')),
>>       ('n', 'number', None, _('list the revision number (default)')),
>>       ('c', 'changeset', None, _('list the changeset')),
>> -    ('l', 'line-number', None, _('show line number at the first
>> appearance'))
>> +    ('l', 'line-number', None, _('show line number at the first
>> appearance')),
>> +    ('', 'skip', [], _('revision to not display (EXPERIMENTAL)'),
>> _('REV')),
>>       ] + diffwsopts + walkopts + formatteropts,
>>       _('[-r REV] [-f] [-a] [-u] [-d] [-n] [-c] [-l] FILE...'),
>>       inferrepo=True)
>> @@ -368,6 +369,10 @@ def annotate(ui, repo, *pats, **opts):
>>       follow = not opts.get('no_follow')
>>       diffopts = patch.difffeatureopts(ui, opts, section='annotate',
>>                                        whitespace=True)
>> +    skiprevs = opts.get('skip')
>> +    if skiprevs:
>> +        skiprevs = scmutil.revrange(repo, skiprevs)
>> +
>>       for abs in ctx.walk(m):
>>           fctx = ctx[abs]
>>           if not opts.get('text') and fctx.isbinary():
>> @@ -375,7 +380,7 @@ def annotate(ui, repo, *pats, **opts):
>>               continue
>>             lines = fctx.annotate(follow=follow, linenumber=linenumber,
>> -                              diffopts=diffopts)
>> +                              skiprevs=skiprevs, diffopts=diffopts)
>>           if not lines:
>>               continue
>>           formats = []
>> diff --git a/mercurial/context.py b/mercurial/context.py
>> --- a/mercurial/context.py
>> +++ b/mercurial/context.py
>> @@ -949,7 +949,8 @@ class basefilectx(object):
>>               return p[1]
>>           return filectx(self._repo, self._path, fileid=-1,
>> filelog=self._filelog)
>>   -    def annotate(self, follow=False, linenumber=False, diffopts=None):
>> +    def annotate(self, follow=False, linenumber=False, skiprevs=None,
>> +                 diffopts=None):
>>           '''returns a list of tuples of ((ctx, number), line) for each
>> line
>>           in the file, where ctx is the filectx of the node where
>>           that line was last changed; if linenumber parameter is true,
>> number is
>> @@ -1044,7 +1045,10 @@ class basefilectx(object):
>>               if ready:
>>                   visit.pop()
>>                   curr = decorate(f.data(), f)
>> -                curr = _annotatepair([hist[p] for p in pl], f, curr,
>> False,
>> +                skipchild = False
>> +                if skiprevs is not None:
>> +                    skipchild = f._changeid in skiprevs
>> +                curr = _annotatepair([hist[p] for p in pl], f, curr,
>> skipchild,
>>                                        diffopts)
>>                   for p in pl:
>>                       if needed[p] == 1:
>> diff --git a/tests/test-annotate.t b/tests/test-annotate.t
>> --- a/tests/test-annotate.t
>> +++ b/tests/test-annotate.t
>> @@ -217,6 +217,77 @@ annotate after rename merge with -l
>>     3 b:5: b5
>>     7 b:7: d
>>   +--skip nothing (should be the same as no --skip at all)
>> +
>> +  $ hg annotate -nlf b --skip '1::0'
>> +  0 a:1: a
>> +  6 b:2: z
>> +  1 a:3: a
>> +  3 b:4: b4
>> +  4 b:5: c
>> +  3 b:5: b5
>> +  7 b:7: d
>> +
>> +--skip a modified line
>> +
>> +  $ hg annotate -nlf b --skip 6
>> +  0 a:1: a
>> +  1 a:2: z
>> +  1 a:3: a
>> +  3 b:4: b4
>> +  4 b:5: c
>> +  3 b:5: b5
>> +  7 b:7: d
>> +
>> +--skip added lines (and test multiple skip)
>> +
>> +  $ hg annotate -nlf b --skip 3
>> +  0 a:1: a
>> +  6 b:2: z
>> +  1 a:3: a
>> +  1 a:3: b4
>> +  4 b:5: c
>> +  1 a:3: b5
>> +  7 b:7: d
>> +
>> +  $ hg annotate -nlf b --skip 4
>> +  0 a:1: a
>> +  6 b:2: z
>> +  1 a:3: a
>> +  3 b:4: b4
>> +  1 a:3: c
>> +  3 b:5: b5
>> +  7 b:7: d
>> +
>> +  $ hg annotate -nlf b --skip 3 --skip 4
>> +  0 a:1: a
>> +  6 b:2: z
>> +  1 a:3: a
>> +  1 a:3: b4
>> +  1 a:3: c
>> +  1 a:3: b5
>> +  7 b:7: d
>> +
>> +  $ hg annotate -nlf b --skip 'merge()'
>> +  0 a:1: a
>> +  6 b:2: z
>> +  1 a:3: a
>> +  3 b:4: b4
>> +  4 b:5: c
>> +  3 b:5: b5
>> +  3 b:5: d
>> +
>> +--skip everything -- use the revision the file was introduced in
>> +
>> +  $ hg annotate -nlf b --skip 'all()'
>> +  0 a:1: a
>> +  0 a:1: z
>> +  0 a:1: a
>> +  0 a:1: b4
>> +  0 a:1: c
>> +  0 a:1: b5
>> +  0 a:1: d
>> +
>>   Issue2807: alignment of line numbers with -l
>>       $ echo more >> b
>> diff --git a/tests/test-completion.t b/tests/test-completion.t
>> --- a/tests/test-completion.t
>> +++ b/tests/test-completion.t
>> @@ -217,7 +217,7 @@ Show an error if we use --options with a
>>   Show all commands + options
>>     $ hg debugcommands
>>     add: include, exclude, subrepos, dry-run
>> -  annotate: rev, follow, no-follow, text, user, file, date, number,
>> changeset, line-number, ignore-all-space, ignore-space-change,
>> ignore-blank-lines, include, exclude, template
>> +  annotate: rev, follow, no-follow, text, user, file, date, number,
>> changeset, line-number, skip, ignore-all-space, ignore-space-change,
>> ignore-blank-lines, include, exclude, template
>>     clone: noupdate, updaterev, rev, branch, pull, uncompressed, ssh,
>> remotecmd, insecure
>>     commit: addremove, close-branch, amend, secret, edit, interactive,
>> include, exclude, message, logfile, date, user, subrepos
>>     diff: rev, change, text, git, binary, nodates, noprefix,
>> show-function, reverse, ignore-all-space, ignore-space-change,
>> ignore-blank-lines, unified, stat, root, include, exclude, subrepos
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Gregory Szorc - May 26, 2017, 3:19 a.m.
On Thu, May 25, 2017 at 4:49 PM, Siddharth Agarwal <sid@less-broken.com>
wrote:

> On 5/24/17 7:39 PM, Siddharth Agarwal wrote:
>
>> # HG changeset patch
>> # User Siddharth Agarwal <sid0@fb.com>
>> # Date 1495679973 25200
>> #      Wed May 24 19:39:33 2017 -0700
>> # Node ID 0c6f6c4b027191c30d867fb4eb0005682ea90a76
>> # Parent  134dcc1222e4b992b4a60c44c9b9ed8d10422632
>> annotate: add a new experimental --skip option to skip revs
>>
>> This option is most useful for mechanical code modifications, especially
>> ones
>> that retain the same number of lines.
>>
>
> BTW, some questions that would be worth addressing at some point:
>
> 1. Should we call it --skip, --ignore or something else? I'm worried that
> "ignore" is too close to hgignore, which is completely unrelated.
>

I like "skip."


> 2. For this to be effective, we'd probably want to have a checked in file
> that can apply a default list of revsets to ignore. What should that file
> be called, and what should be the UI around that look like? Will we want to
> enable this by default with hg blame, and then have a way to disable it? If
> additional revsets are specified with --skip/--ignore, how do those
> interact?
>

.hgannotateskips?

I think the default behavior should be to take the file into account.

I think there should be an argument to disable loading the file.
--ignoreskipsfile or something. However...

Behavior when there is a file and --skip is hard. What we do here may
influence other design decisions. For example, I could imagine someone
wanting to maintain multiple skip files for different scenarios. I could
also imagine the inverse of a skip feature: an "include" feature. You could
use this to say "only annotate revisions that correspond to tagged
revisions" so you could easily see how a file evolved from release to
release. The number of ways to combine revision lists quickly grows out of
control and you need to invent a mini language like revsets so you can
represent all options. So... let's just use revsets? If --skip is given, it
should be the sole source of revisions and there should be a revset
primitive to allow loading a file. That way you can combine a file and a
custom revset any way you want. All of a sudden this looks like mpm's
file-based revsets [and templates] feature...


> 3. git hyper-blame adds a * at the end of each commit that god skipped. Do
> we want to do anything similar? I'm worried that if we turn something like
> that on by default, we'll break tools that parse annotate output.
>

If --skip is used, users are opting in to the new feature and format and I
don't see a major concern changing the format.

If a default file is used, users are opting in to the new feature and
format, kinda. In this case, we may want to preserve BC, but only when
HGPLAIN is set. I'm fine with the BC if HGPLAIN isn't set. But I'm not a
good arbiter of BC decisions :)


>
>> diff --git a/mercurial/commands.py b/mercurial/commands.py
>> --- a/mercurial/commands.py
>> +++ b/mercurial/commands.py
>> @@ -262,7 +262,8 @@ def addremove(ui, repo, *pats, **opts):
>>       ('d', 'date', None, _('list the date (short with -q)')),
>>       ('n', 'number', None, _('list the revision number (default)')),
>>       ('c', 'changeset', None, _('list the changeset')),
>> -    ('l', 'line-number', None, _('show line number at the first
>> appearance'))
>> +    ('l', 'line-number', None, _('show line number at the first
>> appearance')),
>> +    ('', 'skip', [], _('revision to not display (EXPERIMENTAL)'),
>> _('REV')),
>>       ] + diffwsopts + walkopts + formatteropts,
>>       _('[-r REV] [-f] [-a] [-u] [-d] [-n] [-c] [-l] FILE...'),
>>       inferrepo=True)
>> @@ -368,6 +369,10 @@ def annotate(ui, repo, *pats, **opts):
>>       follow = not opts.get('no_follow')
>>       diffopts = patch.difffeatureopts(ui, opts, section='annotate',
>>                                        whitespace=True)
>> +    skiprevs = opts.get('skip')
>> +    if skiprevs:
>> +        skiprevs = scmutil.revrange(repo, skiprevs)
>> +
>>       for abs in ctx.walk(m):
>>           fctx = ctx[abs]
>>           if not opts.get('text') and fctx.isbinary():
>> @@ -375,7 +380,7 @@ def annotate(ui, repo, *pats, **opts):
>>               continue
>>             lines = fctx.annotate(follow=follow, linenumber=linenumber,
>> -                              diffopts=diffopts)
>> +                              skiprevs=skiprevs, diffopts=diffopts)
>>           if not lines:
>>               continue
>>           formats = []
>> diff --git a/mercurial/context.py b/mercurial/context.py
>> --- a/mercurial/context.py
>> +++ b/mercurial/context.py
>> @@ -949,7 +949,8 @@ class basefilectx(object):
>>               return p[1]
>>           return filectx(self._repo, self._path, fileid=-1,
>> filelog=self._filelog)
>>   -    def annotate(self, follow=False, linenumber=False, diffopts=None):
>> +    def annotate(self, follow=False, linenumber=False, skiprevs=None,
>> +                 diffopts=None):
>>           '''returns a list of tuples of ((ctx, number), line) for each
>> line
>>           in the file, where ctx is the filectx of the node where
>>           that line was last changed; if linenumber parameter is true,
>> number is
>> @@ -1044,7 +1045,10 @@ class basefilectx(object):
>>               if ready:
>>                   visit.pop()
>>                   curr = decorate(f.data(), f)
>> -                curr = _annotatepair([hist[p] for p in pl], f, curr,
>> False,
>> +                skipchild = False
>> +                if skiprevs is not None:
>> +                    skipchild = f._changeid in skiprevs
>> +                curr = _annotatepair([hist[p] for p in pl], f, curr,
>> skipchild,
>>                                        diffopts)
>>                   for p in pl:
>>                       if needed[p] == 1:
>> diff --git a/tests/test-annotate.t b/tests/test-annotate.t
>> --- a/tests/test-annotate.t
>> +++ b/tests/test-annotate.t
>> @@ -217,6 +217,77 @@ annotate after rename merge with -l
>>     3 b:5: b5
>>     7 b:7: d
>>   +--skip nothing (should be the same as no --skip at all)
>> +
>> +  $ hg annotate -nlf b --skip '1::0'
>> +  0 a:1: a
>> +  6 b:2: z
>> +  1 a:3: a
>> +  3 b:4: b4
>> +  4 b:5: c
>> +  3 b:5: b5
>> +  7 b:7: d
>> +
>> +--skip a modified line
>> +
>> +  $ hg annotate -nlf b --skip 6
>> +  0 a:1: a
>> +  1 a:2: z
>> +  1 a:3: a
>> +  3 b:4: b4
>> +  4 b:5: c
>> +  3 b:5: b5
>> +  7 b:7: d
>> +
>> +--skip added lines (and test multiple skip)
>> +
>> +  $ hg annotate -nlf b --skip 3
>> +  0 a:1: a
>> +  6 b:2: z
>> +  1 a:3: a
>> +  1 a:3: b4
>> +  4 b:5: c
>> +  1 a:3: b5
>> +  7 b:7: d
>> +
>> +  $ hg annotate -nlf b --skip 4
>> +  0 a:1: a
>> +  6 b:2: z
>> +  1 a:3: a
>> +  3 b:4: b4
>> +  1 a:3: c
>> +  3 b:5: b5
>> +  7 b:7: d
>> +
>> +  $ hg annotate -nlf b --skip 3 --skip 4
>> +  0 a:1: a
>> +  6 b:2: z
>> +  1 a:3: a
>> +  1 a:3: b4
>> +  1 a:3: c
>> +  1 a:3: b5
>> +  7 b:7: d
>> +
>> +  $ hg annotate -nlf b --skip 'merge()'
>> +  0 a:1: a
>> +  6 b:2: z
>> +  1 a:3: a
>> +  3 b:4: b4
>> +  4 b:5: c
>> +  3 b:5: b5
>> +  3 b:5: d
>> +
>> +--skip everything -- use the revision the file was introduced in
>> +
>> +  $ hg annotate -nlf b --skip 'all()'
>> +  0 a:1: a
>> +  0 a:1: z
>> +  0 a:1: a
>> +  0 a:1: b4
>> +  0 a:1: c
>> +  0 a:1: b5
>> +  0 a:1: d
>> +
>>   Issue2807: alignment of line numbers with -l
>>       $ echo more >> b
>> diff --git a/tests/test-completion.t b/tests/test-completion.t
>> --- a/tests/test-completion.t
>> +++ b/tests/test-completion.t
>> @@ -217,7 +217,7 @@ Show an error if we use --options with a
>>   Show all commands + options
>>     $ hg debugcommands
>>     add: include, exclude, subrepos, dry-run
>> -  annotate: rev, follow, no-follow, text, user, file, date, number,
>> changeset, line-number, ignore-all-space, ignore-space-change,
>> ignore-blank-lines, include, exclude, template
>> +  annotate: rev, follow, no-follow, text, user, file, date, number,
>> changeset, line-number, skip, ignore-all-space, ignore-space-change,
>> ignore-blank-lines, include, exclude, template
>>     clone: noupdate, updaterev, rev, branch, pull, uncompressed, ssh,
>> remotecmd, insecure
>>     commit: addremove, close-branch, amend, secret, edit, interactive,
>> include, exclude, message, logfile, date, user, subrepos
>>     diff: rev, change, text, git, binary, nodates, noprefix,
>> show-function, reverse, ignore-all-space, ignore-space-change,
>> ignore-blank-lines, unified, stat, root, include, exclude, subrepos
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>>
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Siddharth Agarwal - May 26, 2017, 3:24 a.m.
On 5/25/17 8:19 PM, Gregory Szorc wrote:
> I could also imagine the inverse of a skip feature: an "include" 
> feature. You could use this to say "only annotate revisions that 
> correspond to tagged revisions" so you could easily see how a file 
> evolved from release to release.


Huh, interesting. I hadn't thought of that.

I think I've been convinced that specifying --skip on the CLI should 
disable reading the defaults file.
Gregory Szorc - May 26, 2017, 5:14 a.m.
On Thu, May 25, 2017 at 8:24 PM, Siddharth Agarwal <sid@less-broken.com>
wrote:

> On 5/25/17 8:19 PM, Gregory Szorc wrote:
>
>> I could also imagine the inverse of a skip feature: an "include" feature.
>> You could use this to say "only annotate revisions that correspond to
>> tagged revisions" so you could easily see how a file evolved from release
>> to release.
>>
>
>
> Huh, interesting. I hadn't thought of that.
>

Following that line of thought, why are we defining this in terms of a
"skip" list? Everything else in Mercurial that speaks in revisions does so
in an inclusive manner. e.g. `hg log` can be thought of as `hg log -r
'all()'`. The feature we're talking about essentially turns `hg annotate`
from "start at <revision> and consider all() revisions that are ancestors"
to "start at <revision> and consider <revset> revisions that are
ancestors," as a skip list is essentially "all() - <skips>." So we /could/
invert the design so we teach `hg annotate` which revisions to "consider"
or are "relevant." `hg annotate -r 123 --from 'all() - desc(backout) -
desc(REFACTOR)'` or something to that effect. The only thing this really
changes is that files defining revsets would have to invert inclusion. e.g.
instead of "X\nY" the file is "all()\n-X\n-Y". That's mildly more annoying,
but arguably more useful and flexible than figuring out semantics when
there is an implicit all() involved.

And I hate to scope bloat, but while we're here, we may want to also
consider how to expose a "reverse blame" (finding a future changeset that
touches a line) feature via the CLI, as some implementations may wish to
combine the concept of an include/skip revset with the direction. e.g. `hg
annotate -r . --from <revset>` and `hg annotate -r . --to <revset>` versus
say `hg annotate -r . --consider <revset> --forward`.

(I haven't fully thought a lot of this through. Just kind of thinking out
loud.)


>
> I think I've been convinced that specifying --skip on the CLI should
> disable reading the defaults file.
>
>
Siddharth Agarwal - May 26, 2017, 9:07 p.m.
On 5/25/17 10:14 PM, Gregory Szorc wrote:
> On Thu, May 25, 2017 at 8:24 PM, Siddharth Agarwal 
> <sid@less-broken.com <mailto:sid@less-broken.com>> wrote:
>
>     On 5/25/17 8:19 PM, Gregory Szorc wrote:
>
>         I could also imagine the inverse of a skip feature: an
>         "include" feature. You could use this to say "only annotate
>         revisions that correspond to tagged revisions" so you could
>         easily see how a file evolved from release to release.
>
>
>
>     Huh, interesting. I hadn't thought of that.
>
>
> Following that line of thought, why are we defining this in terms of a 
> "skip" list? Everything else in Mercurial that speaks in revisions 
> does so in an inclusive manner. e.g. `hg log` can be thought of as `hg 
> log -r 'all()'`. The feature we're talking about essentially turns `hg 
> annotate` from "start at <revision> and consider all() revisions that 
> are ancestors" to "start at <revision> and consider <revset> revisions 
> that are ancestors," as a skip list is essentially "all() - <skips>." 
> So we /could/ invert the design so we teach `hg annotate` which 
> revisions to "consider" or are "relevant." `hg annotate -r 123 --from 
> 'all() - desc(backout) - desc(REFACTOR)'` or something to that effect. 
> The only thing this really changes is that files defining revsets 
> would have to invert inclusion. e.g. instead of "X\nY" the file is 
> "all()\n-X\n-Y". That's mildly more annoying, but arguably more useful 
> and flexible than figuring out semantics when there is an implicit 
> all() involved.

Hmm, so thinking about this a bit more, there are actually two 
completely different use cases here.

1. Large-scale codemods, for which the most useful thing to present here 
is the nearest changeset that's an *ancestor* of a skipped cset.
2. "What changed in <e.g. tag>", for which the most useful thing to 
present is the nearest changeset that's a *descendant* of a skipped cset.

The two cases are quite different, and probably deserve different UI 
surfaces. I'm only solving case 1 for now.

Case 2 definitely doesn't involve any heuristics for linear commits, and 
probably doesn't for branchy history, though I haven't fully worked out 
the latter.
via Mercurial-devel - May 29, 2017, 5:40 a.m.
On Wed, May 24, 2017 at 7:39 PM, Siddharth Agarwal <sid0@fb.com> wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1495679973 25200
> #      Wed May 24 19:39:33 2017 -0700
> # Node ID 0c6f6c4b027191c30d867fb4eb0005682ea90a76
> # Parent  134dcc1222e4b992b4a60c44c9b9ed8d10422632
> annotate: add a new experimental --skip option to skip revs
>
> This option is most useful for mechanical code modifications, especially ones
> that retain the same number of lines.
>
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -262,7 +262,8 @@ def addremove(ui, repo, *pats, **opts):
>      ('d', 'date', None, _('list the date (short with -q)')),
>      ('n', 'number', None, _('list the revision number (default)')),
>      ('c', 'changeset', None, _('list the changeset')),
> -    ('l', 'line-number', None, _('show line number at the first appearance'))
> +    ('l', 'line-number', None, _('show line number at the first appearance')),
> +    ('', 'skip', [], _('revision to not display (EXPERIMENTAL)'), _('REV')),
>      ] + diffwsopts + walkopts + formatteropts,
>      _('[-r REV] [-f] [-a] [-u] [-d] [-n] [-c] [-l] FILE...'),
>      inferrepo=True)
> @@ -368,6 +369,10 @@ def annotate(ui, repo, *pats, **opts):
>      follow = not opts.get('no_follow')
>      diffopts = patch.difffeatureopts(ui, opts, section='annotate',
>                                       whitespace=True)
> +    skiprevs = opts.get('skip')
> +    if skiprevs:
> +        skiprevs = scmutil.revrange(repo, skiprevs)
> +
>      for abs in ctx.walk(m):
>          fctx = ctx[abs]
>          if not opts.get('text') and fctx.isbinary():
> @@ -375,7 +380,7 @@ def annotate(ui, repo, *pats, **opts):
>              continue
>
>          lines = fctx.annotate(follow=follow, linenumber=linenumber,
> -                              diffopts=diffopts)
> +                              skiprevs=skiprevs, diffopts=diffopts)
>          if not lines:
>              continue
>          formats = []
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -949,7 +949,8 @@ class basefilectx(object):
>              return p[1]
>          return filectx(self._repo, self._path, fileid=-1, filelog=self._filelog)
>
> -    def annotate(self, follow=False, linenumber=False, diffopts=None):
> +    def annotate(self, follow=False, linenumber=False, skiprevs=None,
> +                 diffopts=None):
>          '''returns a list of tuples of ((ctx, number), line) for each line
>          in the file, where ctx is the filectx of the node where
>          that line was last changed; if linenumber parameter is true, number is
> @@ -1044,7 +1045,10 @@ class basefilectx(object):
>              if ready:
>                  visit.pop()
>                  curr = decorate(f.data(), f)
> -                curr = _annotatepair([hist[p] for p in pl], f, curr, False,
> +                skipchild = False
> +                if skiprevs is not None:
> +                    skipchild = f._changeid in skiprevs
> +                curr = _annotatepair([hist[p] for p in pl], f, curr, skipchild,
>                                       diffopts)
>                  for p in pl:
>                      if needed[p] == 1:
> diff --git a/tests/test-annotate.t b/tests/test-annotate.t
> --- a/tests/test-annotate.t
> +++ b/tests/test-annotate.t
> @@ -217,6 +217,77 @@ annotate after rename merge with -l
>    3 b:5: b5
>    7 b:7: d
>
> +--skip nothing (should be the same as no --skip at all)
> +
> +  $ hg annotate -nlf b --skip '1::0'
> +  0 a:1: a
> +  6 b:2: z
> +  1 a:3: a
> +  3 b:4: b4
> +  4 b:5: c
> +  3 b:5: b5
> +  7 b:7: d
> +
> +--skip a modified line
> +
> +  $ hg annotate -nlf b --skip 6
> +  0 a:1: a
> +  1 a:2: z
> +  1 a:3: a
> +  3 b:4: b4
> +  4 b:5: c
> +  3 b:5: b5
> +  7 b:7: d

This fails with --pure as follows. I haven't even tried to figure it out.

@@ -232,7 +232,7 @@

   $ hg annotate -nlf b --skip 6
   0 a:1: a
-  1 a:2: z
+  0 a:1: z
   1 a:3: a
   3 b:4: b4
   4 b:5: c

> +
> +--skip added lines (and test multiple skip)
> +
> +  $ hg annotate -nlf b --skip 3
> +  0 a:1: a
> +  6 b:2: z
> +  1 a:3: a
> +  1 a:3: b4
> +  4 b:5: c
> +  1 a:3: b5
> +  7 b:7: d
> +
> +  $ hg annotate -nlf b --skip 4
> +  0 a:1: a
> +  6 b:2: z
> +  1 a:3: a
> +  3 b:4: b4
> +  1 a:3: c
> +  3 b:5: b5
> +  7 b:7: d
> +
> +  $ hg annotate -nlf b --skip 3 --skip 4
> +  0 a:1: a
> +  6 b:2: z
> +  1 a:3: a
> +  1 a:3: b4
> +  1 a:3: c
> +  1 a:3: b5
> +  7 b:7: d
> +
> +  $ hg annotate -nlf b --skip 'merge()'
> +  0 a:1: a
> +  6 b:2: z
> +  1 a:3: a
> +  3 b:4: b4
> +  4 b:5: c
> +  3 b:5: b5
> +  3 b:5: d
> +
> +--skip everything -- use the revision the file was introduced in
> +
> +  $ hg annotate -nlf b --skip 'all()'
> +  0 a:1: a
> +  0 a:1: z
> +  0 a:1: a
> +  0 a:1: b4
> +  0 a:1: c
> +  0 a:1: b5
> +  0 a:1: d
> +
>  Issue2807: alignment of line numbers with -l
>
>    $ echo more >> b
> diff --git a/tests/test-completion.t b/tests/test-completion.t
> --- a/tests/test-completion.t
> +++ b/tests/test-completion.t
> @@ -217,7 +217,7 @@ Show an error if we use --options with a
>  Show all commands + options
>    $ hg debugcommands
>    add: include, exclude, subrepos, dry-run
> -  annotate: rev, follow, no-follow, text, user, file, date, number, changeset, line-number, ignore-all-space, ignore-space-change, ignore-blank-lines, include, exclude, template
> +  annotate: rev, follow, no-follow, text, user, file, date, number, changeset, line-number, skip, ignore-all-space, ignore-space-change, ignore-blank-lines, include, exclude, template
>    clone: noupdate, updaterev, rev, branch, pull, uncompressed, ssh, remotecmd, insecure
>    commit: addremove, close-branch, amend, secret, edit, interactive, include, exclude, message, logfile, date, user, subrepos
>    diff: rev, change, text, git, binary, nodates, noprefix, show-function, reverse, ignore-all-space, ignore-space-change, ignore-blank-lines, unified, stat, root, include, exclude, subrepos
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - May 29, 2017, 1:37 p.m.
On Sun, 28 May 2017 22:40:47 -0700, Martin von Zweigbergk via Mercurial-devel wrote:
> On Wed, May 24, 2017 at 7:39 PM, Siddharth Agarwal <sid0@fb.com> wrote:
> > # HG changeset patch
> > # User Siddharth Agarwal <sid0@fb.com>
> > # Date 1495679973 25200
> > #      Wed May 24 19:39:33 2017 -0700
> > # Node ID 0c6f6c4b027191c30d867fb4eb0005682ea90a76
> > # Parent  134dcc1222e4b992b4a60c44c9b9ed8d10422632
> > annotate: add a new experimental --skip option to skip revs

> > +--skip a modified line
> > +
> > +  $ hg annotate -nlf b --skip 6
> > +  0 a:1: a
> > +  1 a:2: z
> > +  1 a:3: a
> > +  3 b:4: b4
> > +  4 b:5: c
> > +  3 b:5: b5
> > +  7 b:7: d
> 
> This fails with --pure as follows. I haven't even tried to figure it out.
> 
> @@ -232,7 +232,7 @@
> 
>    $ hg annotate -nlf b --skip 6
>    0 a:1: a
> -  1 a:2: z
> +  0 a:1: z
>    1 a:3: a
>    3 b:4: b4
>    4 b:5: c

Perhaps this shows pure/cext bdiff incompatibility. Pure version uses Python
difflib.
Siddharth Agarwal - May 30, 2017, 4:49 p.m.
On 5/29/17 6:37 AM, Yuya Nishihara wrote:
> On Sun, 28 May 2017 22:40:47 -0700, Martin von Zweigbergk via Mercurial-devel wrote:
>> On Wed, May 24, 2017 at 7:39 PM, Siddharth Agarwal <sid0@fb.com> wrote:
>>> # HG changeset patch
>>> # User Siddharth Agarwal <sid0@fb.com>
>>> # Date 1495679973 25200
>>> #      Wed May 24 19:39:33 2017 -0700
>>> # Node ID 0c6f6c4b027191c30d867fb4eb0005682ea90a76
>>> # Parent  134dcc1222e4b992b4a60c44c9b9ed8d10422632
>>> annotate: add a new experimental --skip option to skip revs
>>> +--skip a modified line
>>> +
>>> +  $ hg annotate -nlf b --skip 6
>>> +  0 a:1: a
>>> +  1 a:2: z
>>> +  1 a:3: a
>>> +  3 b:4: b4
>>> +  4 b:5: c
>>> +  3 b:5: b5
>>> +  7 b:7: d
>> This fails with --pure as follows. I haven't even tried to figure it out.
>>
>> @@ -232,7 +232,7 @@
>>
>>     $ hg annotate -nlf b --skip 6
>>     0 a:1: a
>> -  1 a:2: z
>> +  0 a:1: z
>>     1 a:3: a
>>     3 b:4: b4
>>     4 b:5: c
> Perhaps this shows pure/cext bdiff incompatibility. Pure version uses Python
> difflib.

That's what I suspect it is. I'll look at it today, thanks.

- Siddharth

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -262,7 +262,8 @@  def addremove(ui, repo, *pats, **opts):
     ('d', 'date', None, _('list the date (short with -q)')),
     ('n', 'number', None, _('list the revision number (default)')),
     ('c', 'changeset', None, _('list the changeset')),
-    ('l', 'line-number', None, _('show line number at the first appearance'))
+    ('l', 'line-number', None, _('show line number at the first appearance')),
+    ('', 'skip', [], _('revision to not display (EXPERIMENTAL)'), _('REV')),
     ] + diffwsopts + walkopts + formatteropts,
     _('[-r REV] [-f] [-a] [-u] [-d] [-n] [-c] [-l] FILE...'),
     inferrepo=True)
@@ -368,6 +369,10 @@  def annotate(ui, repo, *pats, **opts):
     follow = not opts.get('no_follow')
     diffopts = patch.difffeatureopts(ui, opts, section='annotate',
                                      whitespace=True)
+    skiprevs = opts.get('skip')
+    if skiprevs:
+        skiprevs = scmutil.revrange(repo, skiprevs)
+
     for abs in ctx.walk(m):
         fctx = ctx[abs]
         if not opts.get('text') and fctx.isbinary():
@@ -375,7 +380,7 @@  def annotate(ui, repo, *pats, **opts):
             continue
 
         lines = fctx.annotate(follow=follow, linenumber=linenumber,
-                              diffopts=diffopts)
+                              skiprevs=skiprevs, diffopts=diffopts)
         if not lines:
             continue
         formats = []
diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -949,7 +949,8 @@  class basefilectx(object):
             return p[1]
         return filectx(self._repo, self._path, fileid=-1, filelog=self._filelog)
 
-    def annotate(self, follow=False, linenumber=False, diffopts=None):
+    def annotate(self, follow=False, linenumber=False, skiprevs=None,
+                 diffopts=None):
         '''returns a list of tuples of ((ctx, number), line) for each line
         in the file, where ctx is the filectx of the node where
         that line was last changed; if linenumber parameter is true, number is
@@ -1044,7 +1045,10 @@  class basefilectx(object):
             if ready:
                 visit.pop()
                 curr = decorate(f.data(), f)
-                curr = _annotatepair([hist[p] for p in pl], f, curr, False,
+                skipchild = False
+                if skiprevs is not None:
+                    skipchild = f._changeid in skiprevs
+                curr = _annotatepair([hist[p] for p in pl], f, curr, skipchild,
                                      diffopts)
                 for p in pl:
                     if needed[p] == 1:
diff --git a/tests/test-annotate.t b/tests/test-annotate.t
--- a/tests/test-annotate.t
+++ b/tests/test-annotate.t
@@ -217,6 +217,77 @@  annotate after rename merge with -l
   3 b:5: b5
   7 b:7: d
 
+--skip nothing (should be the same as no --skip at all)
+
+  $ hg annotate -nlf b --skip '1::0'
+  0 a:1: a
+  6 b:2: z
+  1 a:3: a
+  3 b:4: b4
+  4 b:5: c
+  3 b:5: b5
+  7 b:7: d
+
+--skip a modified line
+
+  $ hg annotate -nlf b --skip 6
+  0 a:1: a
+  1 a:2: z
+  1 a:3: a
+  3 b:4: b4
+  4 b:5: c
+  3 b:5: b5
+  7 b:7: d
+
+--skip added lines (and test multiple skip)
+
+  $ hg annotate -nlf b --skip 3
+  0 a:1: a
+  6 b:2: z
+  1 a:3: a
+  1 a:3: b4
+  4 b:5: c
+  1 a:3: b5
+  7 b:7: d
+
+  $ hg annotate -nlf b --skip 4
+  0 a:1: a
+  6 b:2: z
+  1 a:3: a
+  3 b:4: b4
+  1 a:3: c
+  3 b:5: b5
+  7 b:7: d
+
+  $ hg annotate -nlf b --skip 3 --skip 4
+  0 a:1: a
+  6 b:2: z
+  1 a:3: a
+  1 a:3: b4
+  1 a:3: c
+  1 a:3: b5
+  7 b:7: d
+
+  $ hg annotate -nlf b --skip 'merge()'
+  0 a:1: a
+  6 b:2: z
+  1 a:3: a
+  3 b:4: b4
+  4 b:5: c
+  3 b:5: b5
+  3 b:5: d
+
+--skip everything -- use the revision the file was introduced in
+
+  $ hg annotate -nlf b --skip 'all()'
+  0 a:1: a
+  0 a:1: z
+  0 a:1: a
+  0 a:1: b4
+  0 a:1: c
+  0 a:1: b5
+  0 a:1: d
+
 Issue2807: alignment of line numbers with -l
 
   $ echo more >> b
diff --git a/tests/test-completion.t b/tests/test-completion.t
--- a/tests/test-completion.t
+++ b/tests/test-completion.t
@@ -217,7 +217,7 @@  Show an error if we use --options with a
 Show all commands + options
   $ hg debugcommands
   add: include, exclude, subrepos, dry-run
-  annotate: rev, follow, no-follow, text, user, file, date, number, changeset, line-number, ignore-all-space, ignore-space-change, ignore-blank-lines, include, exclude, template
+  annotate: rev, follow, no-follow, text, user, file, date, number, changeset, line-number, skip, ignore-all-space, ignore-space-change, ignore-blank-lines, include, exclude, template
   clone: noupdate, updaterev, rev, branch, pull, uncompressed, ssh, remotecmd, insecure
   commit: addremove, close-branch, amend, secret, edit, interactive, include, exclude, message, logfile, date, user, subrepos
   diff: rev, change, text, git, binary, nodates, noprefix, show-function, reverse, ignore-all-space, ignore-space-change, ignore-blank-lines, unified, stat, root, include, exclude, subrepos