This is an archive of the discontinued Mercurial Phabricator instance.

grep: don't look up copy info unless --follow is given
ClosedPublic

Authored by martinvonz on Jan 16 2019, 8:23 PM.

Details

Summary

If no --follow was given, then the "copy" variable will become
None. In that case we would still look up the copy information from
the filelog and then ignore it. Let's avoid even looking it up.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Jan 16 2019, 8:23 PM
martinvonz updated this revision to Diff 13325.Jan 18 2019, 2:10 PM
This revision was automatically updated to reflect the committed changes.
yuja added a subscriber: yuja.Jan 26 2019, 12:06 AM
  • if fn in skip:

+ copy = None
+ if follow:
+ try:
+ copied = flog.renamed(fnode)
+ except error.WdirUnsupported:
+ copied = ctx[fn].renamed()
+ copy = copied and copied[0]

if copy:
  • skip[copy] = True
  • continue

+ copies.setdefault(rev, {})[fn] = copy
+ if fn in skip:
+ skip[copy] = True
+ continue

files.append(fn)

Did you intentionally move if fn in skip: continue under if follow?
The skip dict has wider scope.

In D5620#83924, @yuja wrote:
  • if fn in skip:

+ copy = None
+ if follow:
+ try:
+ copied = flog.renamed(fnode)
+ except error.WdirUnsupported:
+ copied = ctx[fn].renamed()
+ copy = copied and copied[0]

if copy:
  • skip[copy] = True
  • continue

+ copies.setdefault(rev, {})[fn] = copy
+ if fn in skip:
+ skip[copy] = True
+ continue

files.append(fn)

Did you intentionally move if fn in skip: continue under if follow?
The skip dict has wider scope.

Oops, nope, that wasn't on purpose. Thanks for catching that! Feel free to amend the following to the queued commit if it looks good to you:

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -2955,7 +2955,8 @@ def grep(ui, repo, pattern, *pats, **opt
                     copies.setdefault(rev, {})[fn] = copy
                     if fn in skip:
                         skip[copy] = True
-                        continue
+            if fn in skip:
+                continue
             files.append(fn)

             if fn not in matches[rev]:
yuja added a comment.Jan 26 2019, 2:09 AM
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -2955,7 +2955,8 @@ def grep(ui, repo, pattern, *pats, **opt
                     copies.setdefault(rev, {})[fn] = copy
                     if fn in skip:
                         skip[copy] = True
-                        continue
+            if fn in skip:
+                continue
             files.append(fn)

Applied this, thanks.