This is an archive of the discontinued Mercurial Phabricator instance.

grep: restore pre-9ef10437bb88 behavior, enable wdir search by tweakdefaults
ClosedPublic

Authored by yuja on Jul 11 2018, 10:15 AM.

Details

Summary

Unfortunately, python-hglib relies on the original grep behavior and is
documented as such. Even though we agreed to introduce the BC, we shouldn't
break existing libraries.

So this patch flips the default again and move the new default to
ui.tweakdefaults. We could instead use HGPLAIN to turn this flag off, but
that would be rather confusing as the old/new behaviors are quite different.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

yuja created this revision.Jul 11 2018, 10:15 AM
pulkit accepted this revision.Jul 11 2018, 1:39 PM

So what is the way around this, does this mean grep can't be changed at all
?
Also can't we update hglib to work accordingly.

So what is the way around this, does this mean grep can't be changed at all?
Also can't we update hglib to work accordingly.

I don't know enough about hglib, so can't say about that.

Reason I am accepting this patch is that the current fixing of hg grep is a work in progress and we have not tested that a lot in real life. We should change the deafult once we are fullly confident. Since this is close to release, let's hide the work behind a config option.

This revision was automatically updated to reflect the committed changes.

I think it would make sense to defer the behavior change until we test some more. That said, I do want us to plan to make an intentionally breaking change with the out of the box grep experience *without* tweakdefaults enabled. My reasoning is more or less this: approximately nobody uses hg grep today[0] because its behavior doesn't match what users expect out of the box. I'm willing to call the current behavior a *bug*, and prominently announce that with 4.8 we expect to change the default behavior of hg grep to match user expectations. That's in line with the decision outlined in [1] which is old enough that mpm was part of the decision. I strongly suspect hglib will be the only meaningful breakage, and we can patch hglib to pass the -r 0:tip flag that'll give consistent behavior across all hg versions.

How do we feel about that?

0: Anecdata: it's been run 25 times at work in the last 30 days over our entire userbase, which includes a number of extremely advanced users, and has been renamed to histgrep at Facebook for at least 4 years.
1: https://www.mercurial-scm.org/wiki/GrepPlan

I think it would make sense to defer the behavior change until we test some more. That said, I do want us to plan to make an intentionally breaking change with the out of the box grep experience *without* tweakdefaults enabled. My reasoning is more or less this: approximately nobody uses hg grep today[0] because its behavior doesn't match what users expect out of the box. I'm willing to call the current behavior a *bug*, and prominently announce that with 4.8 we expect to change the default behavior of hg grep to match user expectations. That's in line with the decision outlined in [1] which is old enough that mpm was part of the decision. I strongly suspect hglib will be the only meaningful breakage, and we can patch hglib to pass the -r 0:tip flag that'll give consistent behavior across all hg versions.
How do we feel about that?

A big +1 from my side on that.

0: Anecdata: it's been run 25 times at work in the last 30 days over our entire userbase, which includes a number of extremely advanced users, and has been renamed to histgrep at Facebook for at least 4 years.
1: https://www.mercurial-scm.org/wiki/GrepPlan

yuja added a comment.Jul 12 2018, 9:30 AM
I think it would make sense to defer the behavior change until we test some more. That said, I do want us to plan to make an intentionally breaking change with the out of the box grep experience *without* `tweakdefaults` enabled. My reasoning is more or less this: approximately nobody uses `hg grep` today[0] because its behavior doesn't match what users expect out of the box. I'm willing to call the current behavior a *bug*, and prominently announce that with 4.8 we expect to change the default behavior of `hg grep` to match user expectations. That's in line with the decision outlined in [1] which is old enough that mpm was part of the decision. I strongly suspect hglib will be the only meaningful breakage, and we can patch hglib to pass the -r 0:tip flag that'll give consistent behavior across all hg versions.

If we can take the current hg grep behavior as a bug, we can say the
same thing for python-hglib. I don't want to make hglib grep diverged from
hg one since hglib is a thin wrapper.

If we can take the current hg grep behavior as a bug, we can say the
same thing for python-hglib. I don't want to make hglib grep diverged from
hg one since hglib is a thin wrapper.

That's not a bad thought. Sounds like a plan to me.

How do you feel about treating the current behavior of hg grep as a bug?

yuja added a comment.Jul 13 2018, 8:43 AM
How do you feel about treating the current behavior of `hg grep` as a bug?

I'm 0 on this. 10 years are long enough to turn a bug into a feature, but
I agree the grep (without --all/--diff) is useless. If we had no easy way
to opt-in BC, I would give thumbs up on changing the default behavior.

So what's the next step should be ?

(resend)

So what's the next step should be ?

Maybe add support for "--all-files -rMULTIREV" so the feature can be
documented properly?

https://www.mercurial-scm.org/wiki/GrepPlan#hg_grep
"If given a set of revisions, it will do the same but search the given
revision(s)."