Patchwork D3976: grep: add MULTIREV support to --allfiles flag

login
register
mail settings
Submitter phabricator
Date Aug. 5, 2018, 9:10 p.m.
Message ID <ae70b448b559cd6906072a4d9822a8c6@localhost.localdomain>
Download mbox | patch
Permalink /patch/33281/
State Not Applicable
Headers show

Comments

phabricator - Aug. 5, 2018, 9:10 p.m.
sangeet259 updated this revision to Diff 9937.
sangeet259 edited the summary of this revision.
sangeet259 retitled this revision from "grep: add MULTIREV support to --all-files flag" to "grep: add MULTIREV support to --allfiles flag".

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D3976?vs=9664&id=9937

REVISION DETAIL
  https://phab.mercurial-scm.org/D3976

AFFECTED FILES
  mercurial/cmdutil.py
  mercurial/commands.py
  tests/test-grep.t

CHANGE DETAILS




To: sangeet259, #hg-reviewers
Cc: yuja, mercurial-devel
Yuya Nishihara - Aug. 6, 2018, 1:23 p.m.
Looks mostly good. One nit.

> @@ -2748,7 +2749,7 @@
>          for fn in sorted(revfiles.get(rev, [])):
>              states = matches[rev][fn]
>              copy = copies.get(rev, {}).get(fn)
> -            if fn in skip:
> +            if fn in skip and not all_files:
>                  if copy:
>                      skip[copy] = True
>                  continue

Instead of ignoring `skip[fn]`, it's probably better to not set `skip[fn]`
at all. That's what we do at a couple of lines below, `if r and not diff`.

> @@ -1983,6 +1980,7 @@
>  
>          it = iter(revs)
>          stopiteration = False
> +
>          for windowsize in increasingwindows():
>              nrevs = []
>              for i in xrange(windowsize):
> @@ -1997,14 +1995,18 @@
>                  ctx = change(rev)
>                  if not fns:
>                      def fns_generator():
> +
>                          if allfiles:
>                              fiter = iter(ctx)
>                          else:
> -                            fiter = ctx.files()
> +                            fiter = iter(ctx.files())
>                          for f in fiter:
>                              if match(f):
>                                  yield f
> +
> +
>                      fns = fns_generator()
> +

Can you undo these unrelated changes?
phabricator - Aug. 6, 2018, 1:24 p.m.
yuja added a comment.


  Looks mostly good. One nit.
  
  > @@ -2748,7 +2749,7 @@
  > 
  >   for fn in sorted(revfiles.get(rev, [])):
  >       states = matches[rev][fn]
  >       copy = copies.get(rev, {}).get(fn)
  > 
  > - if fn in skip: +            if fn in skip and not all_files: if copy: skip[copy] = True continue
  
  Instead of ignoring `skip[fn]`, it's probably better to not set `skip[fn]`
  at all. That's what we do at a couple of lines below, `if r and not diff`.
  
  > @@ -1983,6 +1980,7 @@
  > 
  >   it = iter(revs)
  >   stopiteration = False
  > 
  > +
  > 
  >   for windowsize in increasingwindows():
  >       nrevs = []
  >       for i in xrange(windowsize):
  > 
  > @@ -1997,14 +1995,18 @@
  > 
  >   ctx = change(rev)
  >   if not fns:
  >       def fns_generator():
  > 
  > +
  > 
  >   if allfiles:
  >       fiter = iter(ctx)
  >   else:
  > 
  > - fiter = ctx.files() +                            fiter = iter(ctx.files()) for f in fiter: if match(f): yield f + + fns = fns_generator() +
  
  Can you undo these unrelated changes?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3976

To: sangeet259, #hg-reviewers
Cc: yuja, mercurial-devel

Patch

diff --git a/tests/test-grep.t b/tests/test-grep.t
--- a/tests/test-grep.t
+++ b/tests/test-grep.t
@@ -491,3 +491,13 @@ 
   ]
 
   $ cd ..
+
+test -rMULTIREV with --all-files
+
+  $ cd sng
+  $ hg rm um
+  $ hg commit -m "deletes um"
+  $ hg grep -r "0:2" "unmod" --all-files
+  um:0:unmod
+  um:1:unmod
+  $ cd ..
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -2532,6 +2532,7 @@ 
     """
     opts = pycompat.byteskwargs(opts)
     diff = opts.get('all') or opts.get('diff')
+    all_files = opts.get('all_files')
     if diff and opts.get('all_files'):
         raise error.Abort(_('--diff and --all-files are mutually exclusive'))
     # TODO: remove "not opts.get('rev')" if --all-files -rMULTIREV gets working
@@ -2748,7 +2749,7 @@ 
         for fn in sorted(revfiles.get(rev, [])):
             states = matches[rev][fn]
             copy = copies.get(rev, {}).get(fn)
-            if fn in skip:
+            if fn in skip and not all_files:
                 if copy:
                     skip[copy] = True
                 continue
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -1889,9 +1889,6 @@ 
     revs = _walkrevs(repo, opts)
     if not revs:
         return []
-    if allfiles and len(revs) > 1:
-        raise error.Abort(_("multiple revisions not supported with "
-                            "--all-files"))
     wanted = set()
     slowpath = match.anypats() or (not match.always() and opts.get('removed'))
     fncache = {}
@@ -1983,6 +1980,7 @@ 
 
         it = iter(revs)
         stopiteration = False
+
         for windowsize in increasingwindows():
             nrevs = []
             for i in xrange(windowsize):
@@ -1997,14 +1995,18 @@ 
                 ctx = change(rev)
                 if not fns:
                     def fns_generator():
+
                         if allfiles:
                             fiter = iter(ctx)
                         else:
-                            fiter = ctx.files()
+                            fiter = iter(ctx.files())
                         for f in fiter:
                             if match(f):
                                 yield f
+
+
                     fns = fns_generator()
+
                 prepare(ctx, fns)
             for rev in nrevs:
                 yield change(rev)