Patchwork [1,of,3] log: reorganize if-else and for loop in logcmdutil._makematcher()

login
register
mail settings
Submitter Yuya Nishihara
Date Sept. 14, 2020, 1:25 p.m.
Message ID <306c09f80dd9659aa9f0.1600089926@mimosa>
Download mbox | patch
Permalink /patch/47164/
State Accepted
Headers show

Comments

Yuya Nishihara - Sept. 14, 2020, 1:25 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1599804538 -32400
#      Fri Sep 11 15:08:58 2020 +0900
# Node ID 306c09f80dd9659aa9f0eebe1baa328afa3b7977
# Parent  4532e7ebde4d9eb5011b78fb537879bcc67b30c4
log: reorganize if-else and for loop in logcmdutil._makematcher()

The test conditions are branchy depending on --follow and --rev options,
so it should be better to switch first by --follow --rev.

Note that revs is not empty so "if follow and startctxs" can be replaced
with "if follow and opts.get(b'rev')".
Pulkit Goyal - Sept. 16, 2020, 1:01 p.m.
On Mon, Sep 14, 2020 at 6:58 PM Yuya Nishihara <yuya@tcha.org> wrote:
>
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1599804538 -32400
> #      Fri Sep 11 15:08:58 2020 +0900
> # Node ID 306c09f80dd9659aa9f0eebe1baa328afa3b7977
> # Parent  4532e7ebde4d9eb5011b78fb537879bcc67b30c4
> log: reorganize if-else and for loop in logcmdutil._makematcher()
>
> The test conditions are branchy depending on --follow and --rev options,
> so it should be better to switch first by --follow --rev.
>
> Note that revs is not empty so "if follow and startctxs" can be replaced
> with "if follow and opts.get(b'rev')".

Queued the series, many thanks!

Patch

diff --git a/mercurial/logcmdutil.py b/mercurial/logcmdutil.py
--- a/mercurial/logcmdutil.py
+++ b/mercurial/logcmdutil.py
@@ -691,39 +691,47 @@  def _makematcher(repo, revs, pats, opts)
     slowpath = match.anypats() or (not match.always() and opts.get(b'removed'))
     if not slowpath:
         follow = opts.get(b'follow') or opts.get(b'follow_first')
-        startctxs = []
         if follow and opts.get(b'rev'):
             startctxs = [repo[r] for r in revs]
-        for f in match.files():
-            if follow and startctxs:
+            for f in match.files():
                 # No idea if the path was a directory at that revision, so
                 # take the slow path.
                 if any(f not in c for c in startctxs):
                     slowpath = True
                     continue
-            elif follow and f not in wctx:
-                # If the file exists, it may be a directory, so let it
-                # take the slow path.
-                if os.path.exists(repo.wjoin(f)):
-                    slowpath = True
-                    continue
-                else:
-                    raise error.Abort(
-                        _(
-                            b'cannot follow file not in parent '
-                            b'revision: "%s"'
-                        )
-                        % f
-                    )
-            filelog = repo.file(f)
-            if not filelog:
-                # A zero count may be a directory or deleted file, so
-                # try to find matching entries on the slow path.
-                if follow:
+                filelog = repo.file(f)
+                if not filelog:
                     raise error.Abort(
                         _(b'cannot follow nonexistent file: "%s"') % f
                     )
-                slowpath = True
+        elif follow:
+            for f in match.files():
+                if f not in wctx:
+                    # If the file exists, it may be a directory, so let it
+                    # take the slow path.
+                    if os.path.exists(repo.wjoin(f)):
+                        slowpath = True
+                        continue
+                    else:
+                        raise error.Abort(
+                            _(
+                                b'cannot follow file not in parent '
+                                b'revision: "%s"'
+                            )
+                            % f
+                        )
+                filelog = repo.file(f)
+                if not filelog:
+                    raise error.Abort(
+                        _(b'cannot follow nonexistent file: "%s"') % f
+                    )
+        else:
+            for f in match.files():
+                filelog = repo.file(f)
+                if not filelog:
+                    # A zero count may be a directory or deleted file, so
+                    # try to find matching entries on the slow path.
+                    slowpath = True
 
         # We decided to fall back to the slowpath because at least one
         # of the paths was not a file. Check to see if at least one of them
diff --git a/tests/test-log.t b/tests/test-log.t
--- a/tests/test-log.t
+++ b/tests/test-log.t
@@ -2308,6 +2308,20 @@  follow files from wdir
   $ hg log -T '== {rev} ==\n' -fr'wdir()' --git --stat notfound
   notfound: $ENOENT$
 
+follow added/removed files from wdir parent
+
+  $ hg log -T '{rev}\n' -f d1/f2
+  abort: cannot follow nonexistent file: "d1/f2"
+  [255]
+
+  $ hg log -T '{rev}\n' -f f1-copy
+  abort: cannot follow nonexistent file: "f1-copy"
+  [255]
+
+  $ hg log -T '{rev}\n' -f .d6/f1
+  abort: cannot follow file not in parent revision: ".d6/f1"
+  [255]
+
   $ hg revert -aqC
 
 Check that adding an arbitrary name shows up in log automatically