No need to add Edit1, Edit2 in commit message. If something changes from one version to another that deserves mention, include that in commit message as normal.

Perhaps we can start with adding an experimental option to grep files
including unchanged ones?

IIUC, the new default behavior is something like hg grep -r "wdir()" --all-files,
which is basically s/ctx.files()/ctx/.
(needless to say --all-files is a bad name, and the following patch is just a hack.)

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -1835,7 +1835,7 @@ def walkchangerevs(repo, match, opts, pr
     if not revs:
         return []
     wanted = set()
-    slowpath = match.anypats() or (not match.always() and opts.get('removed'))
+    slowpath = True
     fncache = {}
     change = repo.changectx
 
@@ -1889,7 +1889,7 @@ def walkchangerevs(repo, match, opts, pr
                 else:
                     self.revs.discard(value)
                     ctx = change(value)
-                    matches = [f for f in ctx.files() if match(f)]
+                    matches = [f for f in ctx if match(f)]
                     if matches:
                         fncache[value] = matches
                         self.set.add(value)
@@ -1939,7 +1939,7 @@ def walkchangerevs(repo, match, opts, pr
                 ctx = change(rev)
                 if not fns:
                     def fns_generator():
-                        for f in ctx.files():
+                        for f in ctx:
                             if match(f):
                                 yield f
                     fns = fns_generator()

@yuja Can you please clarify this a bit more?

Perhaps we can start with adding an experimental option to grep files
including unchanged ones?"

In D2938#47514, @sangeet259 wrote:

@yuja Can you please clarify this a bit more?

Perhaps we can start with adding an experimental option to grep files
including unchanged ones?"

This patch appears to do 3 separate things by adding a big "if" branch.

• fix grep -r 'wdir()'
• add mode to include unmodified files
• change the default

It's probably better to write one (or more) patches for each change, in a way
not duplicating code for each combination of possible options.

@yuja

fix grep -r 'wdir()'

What exactly do you mean by fixing 'grep -r wdir()' ?

What exactly do you mean by fixing 'grep -r wdir()' ?

Make it return the same result as

$ hg ci -m 'this was wdir()'
$ hg grep -r .

(except the revision number/hash)

@yuja
So I was working on it, but the way I tried to correct "wdir()" is by handling it as a special case, so introducing an "if" block again similar to this.
But you were asking to eliminate the if/else block altogether, I wonder if there is way ?

So I was working on it, but the way I tried to correct "wdir()" is by handling it as a special case, so introducing an "if" block again similar to this.
But you were asking to eliminate the if/else block altogether, I wonder if there is way ?

IIRC, my point was to avoid adding *big* if/else branch which will be likely
to introduce untested code paths.

I don't think there would be a way to go without if/else or try/except as
long as we use the low-level filelog API for performance reasons. Perhaps,
we'll have to catch WdirUnsupported error to fall back to the filectx API.

try:
    flog.read(fnode)
except error.WdirUnsupported:
    ctx[fn].data()

Perhaps, we'll have to catch WdirUnsupported error to fall back to the filectx API.

This seems a good idea.
Trying it.

Hey @yuja
Can you explain why you did this ?

• slowpath = match.anypats() or (not match.always() and opts.get('removed'))

+ slowpath = True

Hey @yuja 
Can you explain why you did this ?
> - slowpath = match.anypats() or (not match.always() and opts.get('removed')) +    slowpath = True

From my vague memory, I guess I would think that if we do s/ctx.files()/ctx/
(which means we're looking for revisions where files exist, not files change),
the use of walkfilerevs() is not valid since filelogs only contain "changes."

To be clear, I'm not saying that will be the desired behavior of hg grep of
"--all-files" of multiple revisions. It might be better to skip intermediate
revisions where files are unchanged.

@yuja I am adding an --unmodified flag to change to change the mode to grep on the unmodified files as well.
What I am trying to do is ::

def fns_generator():
    if --unmodified:
        for f in ctx:
            for f in ctx:
                if match(f):
                    yield f
    else :
        for f in ctx.files():
            for f in ctx:
                if match(f):
                    yield f

Currently, this would work for a single revision, so I am modifying the prep function in grep.
Shall I continue and work on sending the required patch ?

@martinvonz, thanks for the reviews, but @yuja has asked me to break this patch into three patches. So I guess this is not going to be merged anyway.

Currently, I am trying to add a flag like --all_files or --unmodified which will enable one to grep all the files that were tracked by that revision and not just the changed files.

@yuja  I am adding an --unmodified flag to change to change the mode to grep on the unmodified files as well.
What I am trying to do is ::
  def fns_generator():
      if --unmodified:
          for f in ctx:
              for f in ctx:
                  if match(f):
                      yield f
      else :
          for f in ctx.files():
              for f in ctx:
                  if match(f):
                      yield f

Nit: if can be narrowed to ctx/ctx.files().

if --unmodified:
    fiter = iter(ctx)
else:
    fiter = ctx.files()

Currently, this would work for a single revision, so I am modifying the `prep` function in grep.
Shall I continue and send the patch ?

Seems fine. It should be generally encouraged to send patches than discussing
for long without a working code.

but @yuja has asked me to break this patch into three patches. So I guess
this is not going to be merged anyway.

To be clear, I suggested to split patches because the original code seemed
to add a single knob to switch two more features at once.

It looks like this is obsoleted by D3826?

Yeah