Patchwork [RFC] commands: draft a "followlines" command (RFC)

login
register
mail settings
Submitter Denis Laxalde
Date Sept. 28, 2017, 10:04 a.m.
Message ID <0e0fef8d525d9cf60d9c.1506593055@marimba>
Download mbox | patch
Permalink /patch/24201/
State Changes Requested
Headers show

Comments

Denis Laxalde - Sept. 28, 2017, 10:04 a.m.
# HG changeset patch
# User Denis Laxalde <denis@laxalde.org>
# Date 1506589297 -7200
#      Thu Sep 28 11:01:37 2017 +0200
# Node ID 0e0fef8d525d9cf60d9ca23de614e994aca5c9d8
# Parent  2d0c306a88c226d96cf29036b6d6b08bd0abc3b4
# EXP-Topic followlines-cli-rfc
commands: draft a "followlines" command (RFC)

This introduces a new "followlines" command to show history of files by line
range. The command relies on a special syntax to specify file's line range:
FILE,FROMLINE-TOLINE where FILE may be a (rel)path pattern. The command
handles several files and, similarly to the followlines() revset, it also
accepts a --descend and a --startrev option. Most "hg log" options are
supported (with the noticeable exception of --graph, but should be doable).
I'm also willing to add a --rev option and possibly some diff options (ignore
white spaces changes, for instance).

Basically, this command is meant to bring CLI on par with what currently only
exists in hgweb through line selection in "file" and "annotate" views
resulting in a file log with filtered patch to only display followed line
range.

I've thought about extending "hg log" but finally opted for a new command.
Main reasons for this.

* It does not appear feasible (or at least without significant refactoring) to
  adjust the log command to handle the followlines situation in which we have
  to iterate through filectx and line range information, instead of bare
  revisions (obtained from processing log options and file patterns in
  `cmdutil._makelogrevset()`).

* The semantics of this command is arguably a bit different from 'hg log'.
  In particular, when following a block of lines, it seems meaningful to have
  the log displayed in a particular direction (ascending by default, possibly
  descending). Doing this in 'hg log' would require toggling --follow option
  implicitly.

Still, I'm very open to suggestion to embed this in 'hg log'.


The algorithm in followlines command first parses all files and then builds
the map of revision to line range information for all files. Then, revisions
are displayed in ascending or descending order, possibly interleaving file
logs.


In order to filter diff hunks, we pass down a "line range by file name"
mapping (variable `lineranges`) from `cmdutil.changeset_printer.show()` to
`patch.diff()`, where we filter diff hunks corresponding to specified files in
specified line range. Although quite straightforward, this is not a very
elegant approach. I'm open to suggestions here as well (I've thought about
using the matcher to pass the information, but I'm not sure it's
meaningful/appropriate).


Comments welcome, on any aspect.
Augie Fackler - Sept. 30, 2017, 2:06 p.m.
> On Sep 28, 2017, at 11:04, Denis Laxalde <denis@laxalde.org> wrote:
> 
> # HG changeset patch
> # User Denis Laxalde <denis@laxalde.org>
> # Date 1506589297 -7200
> #      Thu Sep 28 11:01:37 2017 +0200
> # Node ID 0e0fef8d525d9cf60d9ca23de614e994aca5c9d8
> # Parent  2d0c306a88c226d96cf29036b6d6b08bd0abc3b4
> # EXP-Topic followlines-cli-rfc
> commands: draft a "followlines" command (RFC)

Patch is awfully big - it'd be helpful to split out some of the precursor work (e.g. stuff in cmdutil.py) into some extra patches that we could land sooner.

> This introduces a new "followlines" command to show history of files by line
> range. The command relies on a special syntax to specify file's line range:
> FILE,FROMLINE-TOLINE where FILE may be a (rel)path pattern. The command
> handles several files and, similarly to the followlines() revset, it also
> accepts a --descend and a --startrev option. Most "hg log" options are
> supported (with the noticeable exception of --graph, but should be doable).
> I'm also willing to add a --rev option and possibly some diff options (ignore
> white spaces changes, for instance).

If we could figure out how to tuck this in 'hg log' I think I'd be happier. Maybe we could do --line-range 'START:END' flag that would only work if exactly one file was specified for 'hg log'?

> Basically, this command is meant to bring CLI on par with what currently only
> exists in hgweb through line selection in "file" and "annotate" views
> resulting in a file log with filtered patch to only display followed line
> range.
> 
> I've thought about extending "hg log" but finally opted for a new command.
> Main reasons for this.
> 
> * It does not appear feasible (or at least without significant refactoring) to
>  adjust the log command to handle the followlines situation in which we have
>  to iterate through filectx and line range information, instead of bare
>  revisions (obtained from processing log options and file patterns in
>  `cmdutil._makelogrevset()`).
> 
> * The semantics of this command is arguably a bit different from 'hg log'.
>  In particular, when following a block of lines, it seems meaningful to have
>  the log displayed in a particular direction (ascending by default, possibly
>  descending). Doing this in 'hg log' would require toggling --follow option
>  implicitly.

Requiring --follow (and defaulting it to on) for --line-range seems eminently reasonable.

> 
> Still, I'm very open to suggestion to embed this in 'hg log'.

Why don't we try and land everything but the top-level command, and then we can look at the options a little more carefully in terms of adding this to log vs adding a new command. Sound good?

I'm excited to have this functionality usable from the command line!
Denis Laxalde - Sept. 30, 2017, 4:39 p.m.
Augie Fackler a écrit :
> 
>> On Sep 28, 2017, at 11:04, Denis Laxalde <denis@laxalde.org> wrote:
>>
>> # HG changeset patch
>> # User Denis Laxalde <denis@laxalde.org>
>> # Date 1506589297 -7200
>> #      Thu Sep 28 11:01:37 2017 +0200
>> # Node ID 0e0fef8d525d9cf60d9ca23de614e994aca5c9d8
>> # Parent  2d0c306a88c226d96cf29036b6d6b08bd0abc3b4
>> # EXP-Topic followlines-cli-rfc
>> commands: draft a "followlines" command (RFC)
> 
> Patch is awfully big - it'd be helpful to split out some of the precursor work (e.g. stuff in cmdutil.py) into some extra patches that we could land sooner.

Yes, sorry about that. I'd certainly split that once I have a proper way
of sorting the situation. Currently, there's not much worth being landed
without the feature...

> 
>> This introduces a new "followlines" command to show history of files by line
>> range. The command relies on a special syntax to specify file's line range:
>> FILE,FROMLINE-TOLINE where FILE may be a (rel)path pattern. The command
>> handles several files and, similarly to the followlines() revset, it also
>> accepts a --descend and a --startrev option. Most "hg log" options are
>> supported (with the noticeable exception of --graph, but should be doable).
>> I'm also willing to add a --rev option and possibly some diff options (ignore
>> white spaces changes, for instance).
> 
> If we could figure out how to tuck this in 'hg log' I think I'd be happier. Maybe we could do --line-range 'START:END' flag that would only work if exactly one file was specified for 'hg log'?

Having this in "hg log" is my preference as well. But I'd rather not
restrict this to only one file if possible. E.g., I'd like to be able to
run a command like:

   hg log mercurial/error.py,43-59 mercurial/error.py,226-247 
mercurial/bundle2.py -r 'date(2014)' -p

where there is a mix file patterns, file patterns with line ranges and
revsets. Hence the syntax with a "," as separator to identify file
patterns with a line range. Anyway, I'll try to have this working and
come back...
Augie Fackler - Sept. 30, 2017, 4:42 p.m.
> On Sep 30, 2017, at 17:39, Denis Laxalde <denis@laxalde.org> wrote:
> 
>>> This introduces a new "followlines" command to show history of files by line
>>> range. The command relies on a special syntax to specify file's line range:
>>> FILE,FROMLINE-TOLINE where FILE may be a (rel)path pattern. The command
>>> handles several files and, similarly to the followlines() revset, it also
>>> accepts a --descend and a --startrev option. Most "hg log" options are
>>> supported (with the noticeable exception of --graph, but should be doable).
>>> I'm also willing to add a --rev option and possibly some diff options (ignore
>>> white spaces changes, for instance).
>> If we could figure out how to tuck this in 'hg log' I think I'd be happier. Maybe we could do --line-range 'START:END' flag that would only work if exactly one file was specified for 'hg log'?
> 
> Having this in "hg log" is my preference as well. But I'd rather not
> restrict this to only one file if possible. E.g., I'd like to be able to
> run a command like:
> 
>  hg log mercurial/error.py,43-59 mercurial/error.py,226-247 mercurial/bundle2.py -r 'date(2014)' -p
> 
> where there is a mix file patterns, file patterns with line ranges and
> revsets. Hence the syntax with a "," as separator to identify file
> patterns with a line range. Anyway, I'll try to have this working and
> come back...

Syntactically how might that work? We notice the specified file doesn't exist, then try doing path.rsplit(',', 1) to see if we get a line range of a file that does exist?

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -1513,7 +1513,7 @@  def export(repo, revs, fntemplate='hg-%h
 
 def diffordiffstat(ui, repo, diffopts, node1, node2, match,
                    changes=None, stat=False, fp=None, prefix='',
-                   root='', listsubrepos=False):
+                   root='', listsubrepos=False, lineranges=None):
     '''show diff or diffstat.'''
     if fp is None:
         write = ui.write
@@ -1541,14 +1541,16 @@  def diffordiffstat(ui, repo, diffopts, n
         if not ui.plain():
             width = ui.termwidth()
         chunks = patch.diff(repo, node1, node2, match, changes, diffopts,
-                            prefix=prefix, relroot=relroot)
+                            prefix=prefix, relroot=relroot,
+                            lineranges=lineranges)
         for chunk, label in patch.diffstatui(util.iterlines(chunks),
                                              width=width):
             write(chunk, label=label)
     else:
         for chunk, label in patch.diffui(repo, node1, node2, match,
                                          changes, diffopts, prefix=prefix,
-                                         relroot=relroot):
+                                         relroot=relroot,
+                                         lineranges=lineranges):
             write(chunk, label=label)
 
     if listsubrepos:
@@ -1610,16 +1612,16 @@  class changeset_printer(object):
         if self.footer:
             self.ui.write(self.footer)
 
-    def show(self, ctx, copies=None, matchfn=None, **props):
+    def show(self, ctx, copies=None, matchfn=None, lineranges=None, **props):
         props = pycompat.byteskwargs(props)
         if self.buffered:
             self.ui.pushbuffer(labeled=True)
-            self._show(ctx, copies, matchfn, props)
+            self._show(ctx, copies, matchfn, props, lineranges)
             self.hunk[ctx.rev()] = self.ui.popbuffer()
         else:
-            self._show(ctx, copies, matchfn, props)
-
-    def _show(self, ctx, copies, matchfn, props):
+            self._show(ctx, copies, matchfn, props, lineranges)
+
+    def _show(self, ctx, copies, matchfn, props, lineranges):
         '''show a single changeset or file revision'''
         changenode = ctx.node()
         rev = ctx.rev()
@@ -1737,14 +1739,14 @@  class changeset_printer(object):
                               label='log.summary')
         self.ui.write("\n")
 
-        self.showpatch(ctx, matchfn)
+        self.showpatch(ctx, matchfn, lineranges=lineranges)
 
     def _exthook(self, ctx):
         '''empty method used by extension as a hook point
         '''
         pass
 
-    def showpatch(self, ctx, matchfn):
+    def showpatch(self, ctx, matchfn, lineranges=None):
         if not matchfn:
             matchfn = self.matchfn
         if matchfn:
@@ -1755,12 +1757,14 @@  class changeset_printer(object):
             prev = ctx.p1().node()
             if stat:
                 diffordiffstat(self.ui, self.repo, diffopts, prev, node,
-                               match=matchfn, stat=True)
+                               match=matchfn, stat=True,
+                               lineranges=lineranges)
             if diff:
                 if stat:
                     self.ui.write("\n")
                 diffordiffstat(self.ui, self.repo, diffopts, prev, node,
-                               match=matchfn, stat=False)
+                               match=matchfn, stat=False,
+                               lineranges=lineranges)
             self.ui.write("\n")
 
 class jsonchangeset(changeset_printer):
@@ -1777,7 +1781,7 @@  class jsonchangeset(changeset_printer):
         else:
             self.ui.write("[]\n")
 
-    def _show(self, ctx, copies, matchfn, props):
+    def _show(self, ctx, copies, matchfn, props, lineranges):
         '''show a single changeset or file revision'''
         rev = ctx.rev()
         if rev is None:
@@ -1851,13 +1855,15 @@  class jsonchangeset(changeset_printer):
             if stat:
                 self.ui.pushbuffer()
                 diffordiffstat(self.ui, self.repo, diffopts, prev, node,
-                               match=matchfn, stat=True)
+                               match=matchfn, stat=True,
+                               lineranges=lineranges)
                 self.ui.write((',\n  "diffstat": "%s"')
                               % j(self.ui.popbuffer()))
             if diff:
                 self.ui.pushbuffer()
                 diffordiffstat(self.ui, self.repo, diffopts, prev, node,
-                               match=matchfn, stat=False)
+                               match=matchfn, stat=False,
+                               lineranges=lineranges)
                 self.ui.write((',\n  "diff": "%s"') % j(self.ui.popbuffer()))
 
         self.ui.write("\n }")
@@ -1911,7 +1917,7 @@  class changeset_templater(changeset_prin
             self.footer += templater.stringify(self.t(self._parts['docfooter']))
         return super(changeset_templater, self).close()
 
-    def _show(self, ctx, copies, matchfn, props):
+    def _show(self, ctx, copies, matchfn, props, lineranges):
         '''show a single changeset or file revision'''
         props = props.copy()
         props.update(templatekw.keywords)
@@ -1943,7 +1949,7 @@  class changeset_templater(changeset_prin
         # write changeset metadata, then patch if requested
         key = self._parts[self._tref]
         self.ui.write(templater.stringify(self.t(key, **props)))
-        self.showpatch(ctx, matchfn)
+        self.showpatch(ctx, matchfn, lineranges=lineranges)
 
         if self._parts['footer']:
             if not self.footer:
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -27,6 +27,7 @@  from . import (
     changegroup,
     cmdutil,
     copies,
+    dagop,
     debugcommands as debugcommandsmod,
     destutil,
     dirstateguard,
@@ -41,6 +42,7 @@  from . import (
     help,
     hg,
     lock as lockmod,
+    match as matchmod,
     merge as mergemod,
     obsolete,
     patch,
@@ -3217,6 +3219,96 @@  def locate(ui, repo, *pats, **opts):
 
     return ret
 
+@command('^followlines|fl',
+    [('', 'descend', False,
+      _('follow changeset history in descending direction')),
+     ('s', 'startrev', '',
+      _('start following from this revision')),
+    ] + [opt for opt in logopts if opt[1] not in ('graph', 'no-merges')],
+    _('[OPTION]... FILE,LRANGE...'),
+    inferrepo=True)
+def followlines(ui, repo, *pats, **opts):
+    """show revision history of files following a block of lines
+
+    Show revision history of specified files and line ranges.
+
+    File arguments must be passed with a line range information as
+    <FILE>,<FROM LINE>-<TO LINE>.
+
+    History following starts from the revision specified by -s/--startrev
+    option and default to workind directory parent if not specified.
+
+    Use --descend to walk history in the descending direction.
+
+    .. container:: verbose
+
+      Examples:
+
+      - changesets touching lines 13 to 23 for file.c::
+
+          hg followlines file.c,13-23
+
+      - changesets touching lines 13 to 23 for file.c and lines 2 to 6 of
+        main.c, starting at revision 1.0, shown in descending direction with
+        patch::
+
+          hg followlines file.c,13-23 main.c,2-6 -s '1.0' --descend
+    """
+    rev = opts.get('startrev')
+    basectx = repo[rev]
+    limit = cmdutil.loglimit(opts)
+
+    def processpats():
+        for pat in pats:
+            if ',' not in pat:
+                raise error.Abort(_("no line range found in %s") % pat)
+            pat, linerange = pat.rsplit(',', 1)
+            kind = matchmod.patkind(pat)
+            if kind not in (None, 'path', 'relpath'):
+                raise error.Abort(
+                    _("pattern kinds other than '(rel)path' not handled")
+                )
+            m = matchmod.match(repo.root, repo.getcwd(), [pat], ctx=basectx)
+            files = [f for f in basectx if m(f)]
+            if len(files) != 1:
+                raise error.ParseError(
+                    # i18n: "followlines" is a keyword
+                    _("followlines expects exactly one file"))
+            fname = files[0]
+            try:
+                fromline, toline = map(int, linerange.split('-'))
+            except ValueError:
+                raise error.Abort(_("invalid line range for %s") % pat)
+            yield fname, util.processlinerange(fromline, toline)
+
+    ctxgenfunc = (dagop.blockdescendants if opts.get('descend')
+                  else dagop.blockancestors)
+
+    # Build a map of useful information by 'ctx' to be later sorted.
+    linerangesbyctx = {}
+    # Call list() in order to catch user errors early.
+    for fname, (fromline, toline) in list(processpats()):
+        fctx = basectx.filectx(fname)
+        for fctx, linerange in ctxgenfunc(fctx, fromline, toline):
+            linerangesbyctx.setdefault(fctx.changectx(), {})[fname] = linerange
+    linerangesbyctx = sorted(linerangesbyctx.iteritems(),
+                             reverse=not opts.get('descend'))
+
+    ui.pager('followlines')
+    displayer = cmdutil.show_changeset(ui, repo, opts, buffered=True)
+    count = 0
+    for ctx, lineranges in linerangesbyctx:
+        matchfn = matchmod.match(repo.root, repo.getcwd(),
+                                 list(lineranges), ctx=basectx)
+        # TODO handle copies and renamed
+        displayer.show(ctx, matchfn=matchfn,
+                       lineranges=lineranges)
+        if displayer.flush(ctx):
+            count += 1
+        if limit and count == limit:
+            break
+    displayer.close()
+
 @command('^log|history',
     [('f', 'follow', None,
      _('follow changeset history, or file history across copies and renames')),
diff --git a/mercurial/patch.py b/mercurial/patch.py
--- a/mercurial/patch.py
+++ b/mercurial/patch.py
@@ -2294,7 +2294,8 @@  def difffeatureopts(ui, opts=None, untru
     return mdiff.diffopts(**pycompat.strkwargs(buildopts))
 
 def diff(repo, node1=None, node2=None, match=None, changes=None,
-         opts=None, losedatafn=None, prefix='', relroot='', copy=None):
+         opts=None, losedatafn=None, prefix='', relroot='', copy=None,
+         lineranges=None):
     '''yields diff of changes to files between two nodes, or node and
     working directory.
 
@@ -2316,14 +2317,24 @@  def diff(repo, node1=None, node2=None, m
     patterns that fall outside it will be ignored.
 
     copy, if not empty, should contain mappings {dst@y: src@x} of copy
-    information.'''
-    for header, hunks in diffhunks(repo, node1=node1, node2=node2, match=match,
-                                   changes=changes, opts=opts,
-                                   losedatafn=losedatafn, prefix=prefix,
-                                   relroot=relroot, copy=copy):
+    information.
+
+    lineranges, if not None, must be a mapping from filename to line range
+    tuple and is used to filter diff hunks not in specified range.
+    '''
+    for hdr, hunks in diffhunks(repo, node1=node1, node2=node2, match=match,
+                                changes=changes, opts=opts,
+                                losedatafn=losedatafn, prefix=prefix,
+                                relroot=relroot, copy=copy):
+        if lineranges is not None and hdr:
+            fname = header(hdr).filename()
+            linerange = lineranges.get(fname)
+            if linerange is not None:
+                hunks = ((hrange, hlines) for hrange, hlines in hunks
+                         if mdiff.hunkinrange(hrange[2:], linerange))
         text = ''.join(sum((list(hlines) for hrange, hlines in hunks), []))
-        if header and (text or len(header) > 1):
-            yield '\n'.join(header) + '\n'
+        if hdr and (text or len(hdr) > 1):
+            yield '\n'.join(hdr) + '\n'
         if text:
             yield text