Patchwork D3763: grep: add `--diff` flag

login
register
mail settings
Submitter phabricator
Date June 17, 2018, 10:31 a.m.
Message ID <differential-rev-PHID-DREV-5sofkowpz3z3htqasc4v-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/32215/
State Superseded
Headers show

Comments

phabricator - June 17, 2018, 10:31 a.m.
sangeet259 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  adds a diff flag, which works exactly same as all, infact since
  -all searches diffs, there diff is a better name for it.
  The all flag is still here for backward compatibility reasons.
  Some major tests for all has been picked and added for diff.

REPOSITORY
  rHG Mercurial

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

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

CHANGE DETAILS




To: sangeet259, #hg-reviewers
Cc: mercurial-devel
phabricator - June 18, 2018, 8:15 p.m.
pulkit added inline comments.

INLINE COMMENTS

> commands.py:2419
>      ('', 'all', None, _('print all revisions that match')),
> +    ('', 'diff', None, _('print all revisions that match')),
>      ('a', 'text', None, _('treat all files as text')),

We definitely need better help text here to mention that it looks at the diff not all the content.

> commands.py:2446
>      a non-match, or "+" for a non-match that becomes a match), use the
> -    --all flag.
> +    --diff/--all(DEPRECATED) flag.
>  

To deprecate a flag, we need to add (DEPRECATED) after the help text of the flag. Look for DEPRECATED, EXPERIMENTAL, ADVANCED in commands.py, you will find examples.

Also, I suggest you to make a separate patch for deprecating the flag.

> commands.py:2554
>          fieldnamemap = {'filename': 'file', 'linenumber': 'line_number'}
> -        if opts.get('all'):
> +        if opts.get('all') or opts.get('diff'):
>              iter = difflinestates(pstates, states)

nit: it will be better if we can initialize a variable and store `opts.get('all') and opts.get('diff')` in it and reuse that.

REPOSITORY
  rHG Mercurial

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

To: sangeet259, #hg-reviewers
Cc: pulkit, 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
@@ -281,6 +281,11 @@ 
   color:2:-:orange
   color:1:+:orange
 
+  $ hg grep --diff orange
+  color:3:+:orange
+  color:2:-:orange
+  color:1:+:orange
+
 test substring match: '^' should only match at the beginning
 
   $ hg grep '^.' --config extensions.color= --color debug
@@ -349,6 +354,10 @@ 
   color:3:-:red
   color:1:+:red
 
+  $ hg grep --diff red
+  color:3:-:red
+  color:1:+:red
+
 Issue3885: test that changing revision order does not alter the
 revisions printed, just their order.
 
@@ -360,6 +369,14 @@ 
   color:3:-:red
   color:1:+:red
 
+  $ hg grep --diff red -r "all()"
+  color:1:+:red
+  color:3:-:red
+
+  $ hg grep --diff red -r "reverse(all())"
+  color:3:-:red
+  color:1:+:red
+
   $ cd ..
 
   $ hg init a
@@ -370,6 +387,9 @@ 
   $ hg grep "MaCam" --all
   binfile.bin:0:+: Binary file matches
 
+  $ hg grep "MaCam" --diff
+  binfile.bin:0:+: Binary file matches
+
   $ cd ..
 
 Fix_Wdir(): test that passing wdir() t -r flag does greps on the
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -2416,6 +2416,7 @@ 
 @command('grep',
     [('0', 'print0', None, _('end fields with NUL')),
     ('', 'all', None, _('print all revisions that match')),
+    ('', 'diff', None, _('print all revisions that match')),
     ('a', 'text', None, _('treat all files as text')),
     ('f', 'follow', None,
      _('follow changeset history,'
@@ -2442,7 +2443,7 @@ 
     file in which it finds a match. To get it to print every revision
     that contains a change in match status ("-" for a match that becomes
     a non-match, or "+" for a non-match that becomes a match), use the
-    --all flag.
+    --diff/--all(DEPRECATED) flag.
 
     PATTERN can be any Python (roughly Perl-compatible) regular
     expression.
@@ -2550,7 +2551,7 @@ 
                 return ctx[fn].isbinary()
 
         fieldnamemap = {'filename': 'file', 'linenumber': 'line_number'}
-        if opts.get('all'):
+        if opts.get('all') or opts.get('diff'):
             iter = difflinestates(pstates, states)
         else:
             iter = [('', l) for l in states]
@@ -2563,7 +2564,7 @@ 
                 ('rev', rev, True),
                 ('linenumber', l.linenum, opts.get('line_number')),
             ]
-            if opts.get('all'):
+            if opts.get('all') or opts.get('diff'):
                 cols.append(('change', change, True))
             cols.extend([
                 ('user', formatuser(ctx.user()), opts.get('user')),
@@ -2667,7 +2668,7 @@ 
             if pstates or states:
                 r = display(fm, fn, ctx, pstates, states)
                 found = found or r
-                if r and not opts.get('all'):
+                if r and not (opts.get('all') or opts.get('diff')):
                     skip[fn] = True
                     if copy:
                         skip[copy] = True