| Submitter | phabricator |
|---|---|
| Date | June 13, 2018, 12:09 p.m. |
| Message ID | <differential-rev-PHID-DREV-mgnd4fiy4rp6mj44nxx3-req@phab.mercurial-scm.org> |
| Download | mbox | patch |
| Permalink | /patch/32105/ |
| State | Superseded |
| Headers | show |
Comments
sangeet259 added a comment. I am updating the tests REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3728 To: sangeet259, #hg-reviewers Cc: mercurial-devel
mharbison72 added inline comments. INLINE COMMENTS > commands.py:2411 > _('only search files changed within revision range'), _('REV')), > + ('', 'unmodified', False, > + _('include all files in the changeset while grepping')), I wonder if `--allfiles` is a better name. `--unmodified` makes me think unmodified files exclusively. It's too bad `--all` is already used, because that would be consistent with the all files meaning in `status` and `revert`. (Maybe others?) The plan page mentions BCing plain grep into oblivion, and deprecating `--all`. Is it worth a BC on this too, for consistency going forward? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3728 To: sangeet259, #hg-reviewers Cc: mharbison72, mercurial-devel
> > commands.py:2411 > > _('only search files changed within revision range'), _('REV')), > > + ('', 'unmodified', False, > > + _('include all files in the changeset while grepping')), > > I wonder if `--allfiles` is a better name. `--unmodified` makes me think unmodified files exclusively. > > It's too bad `--all` is already used, because that would be consistent with the all files meaning in `status` and `revert`. (Maybe others?) The plan page mentions BCing plain grep into oblivion, and deprecating `--all`. Is it worth a BC on this too, for consistency going forward? Maybe we can keep all new flags hidden as "(EXPERIMENTAL)" until we make the final BC? I don't think it's time for bikeshedding yet.
yuja added a comment. >> commands.py:2411 > > _('only search files changed within revision range'), _('REV')), > > + ('', 'unmodified', False, > > + _('include all files in the changeset while grepping')), > > I wonder if `--allfiles` is a better name. `--unmodified` makes me think unmodified files exclusively. > > It's too bad `--all` is already used, because that would be consistent with the all files meaning in `status` and `revert`. (Maybe others?) The plan page mentions BCing plain grep into oblivion, and deprecating `--all`. Is it worth a BC on this too, for consistency going forward? Maybe we can keep all new flags hidden as "(EXPERIMENTAL)" until we make the final BC? I don't think it's time for bikeshedding yet. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3728 To: sangeet259, #hg-reviewers Cc: yuja, mharbison72, mercurial-devel
pulkit added a comment. In https://phab.mercurial-scm.org/D3728#58540, @yuja wrote: > >> commands.py:2411 > > > _('only search files changed within revision range'), _('REV')), > > > + ('', 'unmodified', False, > > > + _('include all files in the changeset while grepping')), > > > > I wonder if `--allfiles` is a better name. `--unmodified` makes me think unmodified files exclusively. > > > > It's too bad `--all` is already used, because that would be consistent with the all files meaning in `status` and `revert`. (Maybe others?) The plan page mentions BCing plain grep into oblivion, and deprecating `--all`. Is it worth a BC on this too, for consistency going forward? > > Maybe we can keep all new flags hidden as "(EXPERIMENTAL)" until we make > the final BC? I don't think it's time for bikeshedding yet. I agree with Yuya and Matt both. How about renaming the flag to `--allfiles` and mark that as experimental. To mark a flag as experimental, append the help text with '(EXPERIMENTAL)'. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3728 To: sangeet259, #hg-reviewers Cc: pulkit, yuja, mharbison72, mercurial-devel
mharbison72 added a comment. In https://phab.mercurial-scm.org/D3728#58540, @yuja wrote: > >> commands.py:2411 > > > _('only search files changed within revision range'), _('REV')), > > > + ('', 'unmodified', False, > > > + _('include all files in the changeset while grepping')), > > > > I wonder if `--allfiles` is a better name. `--unmodified` makes me think unmodified files exclusively. > > > > It's too bad `--all` is already used, because that would be consistent with the all files meaning in `status` and `revert`. (Maybe others?) The plan page mentions BCing plain grep into oblivion, and deprecating `--all`. Is it worth a BC on this too, for consistency going forward? > > Maybe we can keep all new flags hidden as "(EXPERIMENTAL)" until we make > the final BC? I don't think it's time for bikeshedding yet. Agreed. I haven't paid close attention to this, so I wasn't sure what was left. But I wanted to flag it before I forgot. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3728 To: sangeet259, #hg-reviewers Cc: pulkit, yuja, mharbison72, mercurial-devel
Yuya Nishihara <yuya@tcha.org> writes: >> > commands.py:2411 >> > _('only search files changed within revision range'), _('REV')), >> > + ('', 'unmodified', False, >> > + _('include all files in the changeset while grepping')), >> >> I wonder if `--allfiles` is a better name. `--unmodified` makes me think unmodified files exclusively. >> >> It's too bad `--all` is already used, because that would be consistent with the all files meaning in `status` and `revert`. (Maybe others?) The plan page mentions BCing plain grep into oblivion, and deprecating `--all`. Is it worth a BC on this too, for consistency going forward? > > Maybe we can keep all new flags hidden as "(EXPERIMENTAL)" until we make > the final BC? I don't think it's time for bikeshedding yet. Yeah, that sounds like a good way forward to me.
pulkit added a comment. Looks like you forgot to update the commit message. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3728 To: sangeet259, #hg-reviewers Cc: pulkit, yuja, mharbison72, mercurial-devel
sangeet259 added a comment. @yuja Sorry I didn't run the tests after this edit . REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3728 To: sangeet259, #hg-reviewers Cc: pulkit, yuja, mharbison72, mercurial-devel
Patch
diff --git a/tests/test-grep.t b/tests/test-grep.t --- a/tests/test-grep.t +++ b/tests/test-grep.t @@ -368,3 +368,15 @@ binfile.bin:0:+: Binary file matches $ cd .. + +Test for showing working of unmodified flag + + $ hg init sng + $ cd sng + $ echo "unmod" >> um + $ hg ci -A -m "adds unmod to um" + adding um + $ hg grep -r "-1" "unmod" + um:0:unmod + + $ cd .. diff --git a/mercurial/commands.py b/mercurial/commands.py --- a/mercurial/commands.py +++ b/mercurial/commands.py @@ -2408,6 +2408,8 @@ ('n', 'line-number', None, _('print matching line numbers')), ('r', 'rev', [], _('only search files changed within revision range'), _('REV')), + ('', 'unmodified', False, + _('include all files in the changeset while grepping')), ('u', 'user', None, _('list the author (long with -v)')), ('d', 'date', None, _('list the date (short with -q)')), ] + formatteropts + walkopts, diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py --- a/mercurial/cmdutil.py +++ b/mercurial/cmdutil.py @@ -1881,6 +1881,7 @@ yielding each context, the iterator will first call the prepare function on each context in the window in forward order.''' + unmodified = opts.get('unmodified') follow = opts.get('follow') or opts.get('follow_first') revs = _walkrevs(repo, opts) if not revs: @@ -1990,7 +1991,11 @@ ctx = change(rev) if not fns: def fns_generator(): - for f in ctx.files(): + if unmodified and len(revs) == 1: + fiter = iter(ctx) + else: + fiter = ctx.files() + for f in fiter: if match(f): yield f fns = fns_generator()