sangeet259 added a comment.
@yuja Can you please clarify this a bit more? > Perhaps we can start with adding an experimental option to grep files > including unchanged ones?" INLINE COMMENTS > yuja wrote in commands.py:2474 > Better to test if ctx is a workingctx (i.e. `ctx.rev() is None`). > > Another idea is to make `wctx.filenode()` return wdirid and catch > WdirUnsupported exception > to fall back to the slow path. Sure, pushing the first approach now. But, I think the second approach fits the "Easier to ask for forgiveness than permission" philosophy of python. Currently the filenode of workingctx returns this: `return self._fileinfo(path)[0]` . Shall I change that to returning wdirid? Also can you please highlight the slowpath part? Any reference or links to mailing lists, where I can learn more about it. > yuja wrote in commands.py:2475 > Nit: `fctx.isbinary()` is preferred. Done. Any reason why that is better? > av6 wrote in commands.py:2494 > Since the difference in both branches for this if-else block seems to be only > in this line, and just one value, it probably makes sense to store said value > in a variable and use it in place of False/True and reduce duplication. Great. Thanks ! > pulkit wrote in commands.py:2584 > The above if condition wants opts['rev'] to be empty and here you are using > it's value. I could have made the call by simply passing `''` instead of `opts.get('rev')` but I chose this because I am planning to build upon this to handle revisions as well. > pulkit wrote in test-grep.t:357 > what's this None here? The None was a result of displaying revision in the ui.write. Removed that. I had forgotten to update this result. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2938 To: sangeet259, #hg-reviewers Cc: av6, yuja, pulkit, mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel