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

Gregory Szorc gregory.szorc at gmail.com
Thu May 25 23:19:51 EDT 2017


On Thu, May 25, 2017 at 4:49 PM, Siddharth Agarwal <sid at less-broken.com>
wrote:

> On 5/24/17 7:39 PM, Siddharth Agarwal wrote:
>
>> # HG changeset patch
>> # User Siddharth Agarwal <sid0 at 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 at mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>>
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170525/418fffcf/attachment.html>


More information about the Mercurial-devel mailing list