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