The commit message has diff which confuses hg import. You need to indent those diffs.

✅

Also, remove the pastebin links as they are not required since we have diffs.

✅

Download this bundle : http://bit.ly/2DuJjsS

This link asks me to login. So definitely not a good link to put in commit message.

✅

@pulkit

My understanding is that the 1st patch should be present in the mercurial installed in the system, which is what you said right? If that's correct, we can go ahead and have this whole as one patch as I don't think people usually do install system hg on patch to patch basis.

In D6356#92644, @pulkit wrote:

Why do I need to split the patches into two ?
The reason to split what would have been a single patch into two is because
for the next patch to be able to use the mdiff during tests,
this revision has to be there in the system's mercurial installation.

I didn't get the last line. Can you explain more?
I think both can be folded in one and should be good.

Oh, nice! I sometimes use run-tests.py -i and just accept an unreadable diff because I know that hg diff might make it easier to read. I had not understood why :)

Glad to know you will be helped :)

In D6356#92451, @martinvonz wrote:

The future changes to run-tests.py will enable one to use mdiif
to diff rather than unidiff whenever the mercurial is installed in the system.

Perhaps a naive question, but why is that desirable? How will I (as someone running tests) notice?

Apologies, I didn't run the tests after the latest addition. 😔

Hey @durin42 , take a look at the changes when you get time.

@durin42 So the try/except will fall back to unified diff?
Is there a way we can enforce this on system's that don't have mercurial installed globally and not have to fall back on the earlier practice.

Note: To see this in action, one needs to add the changes from this mdiff.py to the mdiff.py of their installed mercurial.
This is because unless you do that, you cannot import mdiff.new_patch as that function won't be there in your installed version, and hence it will keep falling back to the unified_diff.

@yuja
This passes the tests now. I had missed the subsequent code that was dependent on the earlier initialisation of self._testdir.
Fixed it.

I passed the wrong revision number in the -r flag.

This was an error. Ignore, something went wrong.

In D3976#64274, @yuja wrote:

Queued with minor cleanup, thanks.
As a follow up, can you fix hg grep --all-files -rREVS FILE to scan
unchanged revisions?

@yuja Cool, sending the option C now, will try out option D and send a patch if something comes up.

I also expect hg grep --all-files -r0+1 foo will show matches from both rev 0 and 1.

Suppose there are ten hits in 0 and the same 10 hits in 1, do you mean we print out all the 20 results, What purpose that would serve?

Keeping all ctx objects might use too much memory.

True I will change it to files[f] = True

I don't think we should omit files that were seen in earlier revisions

If I don't skip that would mean the same file in the same state being searched across all the revisions, and getting repetitive and redundant hits.

@yuja can you review this one

I created a new revision instead of updating the D3976, ignore this one.

So what's the next step should be ?

So what is the way around this, does this mean grep can't be changed at all
?
Also can't we update hglib to work accordingly.

Yeah

I am sorry I accidentally pushed here :/

I need some help with the tests failing here.

@yuja Sorry I didn't run the tests after this edit .

I am updating the tests

@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.

@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

Hey @yuja
Can you explain why you did this ?

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

@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 ?

@yuja

fix grep -r 'wdir()'