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

[Augie Fackler]

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.

>
> As I expressed the first time this was discussed I think we should start by
> introducing a way to revert the direction during the revert itself so that
> people switch to "apply-diff" when needed without touching the default
> behavior.

I don't understand this proposal, so maybe you could reply-all with a
sample invocation dummied out for us to consider?

> This is a smaller step without negative impact, if after a while user are
> pressing for changing the default, we could revisit the question.

I'm still strongly inclined to take this patch, because you're the
only one opposed.

>
> Cheers,
>
> --
> Pierre-Yves David
>
> [1] that one is unchanged in this patch.
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel