D2938: grep: make grep search on working directory by default
sangeet259 (Sangeet Kumar Mishra)
phabricator at mercurial-scm.org
Sun Mar 25 03:28:09 EDT 2018
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
More information about the Mercurial-devel
mailing list