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

[Sean Farley]

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.
>>
>> 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.