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

Reply via email to