[PATCH RESEND] revert: do not reverse hunks in interactive when REV is not parent (issue5096)

[Martin von Zweigbergk]

On Thu, Nov 10, 2016 at 3:20 PM, Martin von Zweigbergk
<martinvonz at google.com> wrote:
> On Thu, Nov 10, 2016 at 10:11 AM, Sean Farley <sean at farley.io> wrote:
>> Augie Fackler <raf at durin42.com> writes:
>>
>>> On Thu, Nov 10, 2016 at 9:19 AM, Pierre-Yves David
>>> <pierre-yves.david at ens-lyon.org> wrote:
>>>> On 11/07/2016 03:17 PM, Denis Laxalde wrote:
>>>>>
>>>>> # HG changeset patch
>>>>> # User Denis Laxalde <denis.laxalde at logilab.fr>
>>>>> # Date 1475237490 -7200
>>>>> #      Fri Sep 30 14:11:30 2016 +0200
>>>>> # Node ID 4f7b7750403ce48e78d0f361236f65ac03584c3c
>>>>> # Parent  d06c049695e6ad3219e7479c65ce98a2f123e878
>>>>> revert: do not reverse hunks in interactive when REV is not parent
>>>>> (issue5096)
>>>>
>>>>
>>>> (note, we should make this a BC)
>>>>
>>>>>
>>>>> And introduce a new "apply" operation verb for this case as suggested in
>>>>> issue5096. This replaces the no longer used "revert" operation.
>>>>>
>>>>> In interactive revert, when reverting to something else that the parent
>>>>> revision, display an "apply this change" message with a diff that is not
>>>>> reversed.
>>>>>
>>>>> The rationale is that `hg revert -i -r REV` will show hunks of the diff
>>>>> from
>>>>> the working directory to REV and prompt the user to select them for
>>>>> applying
>>>>> (to working directory). This somehow contradicts dcc56e10c23b in which it
>>>>> was
>>>>> decided to have the "direction" of prompted hunks reversed...
>>>>
>>>>
>>>> I still think this is not the right thing to
>>>>
>>>> To me, Having `hg diff`, `hg commit/amend/shelve -i` and `hg revert` showing
>>>> the same hunk make sense and is really useful. In particular showing the
>>>> change I'm about to revert makes a lot of sense when I revert to a parent¹
>>>> or an ancestors, or anything in the background of the patch (precursors, or
>>>> ancestors, or combination of these). I check the change I introduce since a
>>>> given point and revert some. To me using a revert to fetch content from
>>>> arbitrary unrelated changeset is a semantic drift of the command. Such
>>>> semantic drift should not motivate change that alter the core goal of the
>>>> commands.
>
> I do agree with this point. How about introducing a "hg apply -r $rev"
> that's a synonym for "hg revert -r $rev", except that it uses the
> "apply" direction in interactive mode? I was positively surprised that
> "hg apply" is not yet taken. Then we can consider even rejecting "hg
> revert -ir $rev" unless $rev is ".".

No other voices were raised, and JordiGH says "revert" sounds right to
him, so patch queued. Thanks!

>
>>>>
>>>> Having a different behavior depending of the target (with or without -r, -r
>>>> on a background changeset or not) seems too confusing for me. The change in
>>>> the action verb (revert/apply) seems too subtle to me this is going to bite
>>>> user.
>>>
>>> I've got 2 committers beyond me that agree the current behavior of
>>> revert is confusing, plus at least one contributor from Logilab. I
>>> realize you worry this is confusing, but I have had to experiment
>>> multiple times on a test repo before being able to safely use 'hg
>>> revert -ir' and this patch resolves that usability problem for me.
>>> Today, we say "discard" or "revert" based on the value of -r already,
>>> but the word "revert" has to be parsed and then understood to mean
>>> "I'm going to patch -R this if you say yes". The double negative
>>> requires careful thought on my part, and has the same net meaning.
>>
>> Yeah, I agree with Augie here. Wholeheartedly, in fact.