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

[Denis Laxalde]

Pierre-Yves David a écrit :
> We can extract two main points in the section you reply to and I'm not
> sure which one you address in your reply. I'm going to re-emit them in a
> simplified/modified form to clarify
>
> First important point
> =====================
>
> In the following case:
> * `hg revert -i`
> * `hg revert -i -r ancestor`
> * `hg revert -i -r precursor`
> * `hg revert -i -r background` #combination of the above,
>
> Seeing the hunk the way they are currently is exactly what I want to
> see. These hunks are shown that way by all other commands (diff, commit,
> amend, shelve) and not seeing the same thing in revert is super
> confusing to me.

There's an essential different with these "other commands"
(commit/shelve) and revert: the former operate on the repository history
while the latter operates on the working directory. In this respect,
when interactively applying changes to the repository (commit-like
commands, former case), one gets prompted with a diff as it would apply
to the current revision. This patch essentially proposes to apply the
same logic to the revert command (i.e. prompt the user with a change
that would be applied to the working directory) and to prompt the user
with an intelligible action (avoid double negation).

For me, as a user, REV being in the "background" or not is not
particularly relevant. I sometimes use -r REV with REV being on some
"unrelated" branch. More often do I use -r REV with REV being a
descendant changeset (for instance to extract a refactoring that I
committed together with a functional change -- hg up REV^; hg rev -i -r
REV; then commit/rebase). I can't figure out how your reasoning would
apply in this case, but I'm certain that being prompted with changes as
they would apply to the working directory won't be confusing for me.

>  (typical, (use hg diff, see something to drop; use hg
> revert, drop the same thing at the same spot).

That one (which corresponds to `hg revert -i [-r .]`) already works that
way, only it shows a "discard this hunk" prompt.


> An excellent example for such case is `hg revert -i -r .~1`, associate
> with `hg pdif `(hg diff -r .~1) and amend I'm a bit user of it.
>
> In that case, I'm not "worried it will be confusing" I know from
> experience that I'm greatly confused by it. This was one of the
> motivation of dcc56e10c23b;
>
> The fact "hg revert -i" display the right thing in these situation is
> partly validated by the fact this patch for not change the direction for
> the first item in my list (plain `hg revert -i`)

Without -r REV, `hg revert -i` is basically a "discard uncommitted
changes" command. So ones gets prompted with these uncommitted changes
and whether to discard them or not. Sure the diff is reversed with
respected to other cases, but the fundamental idea is that the
combination of "diff direction" and "operation" (discard or apply) is
consistent.

> The second import point
> =======================
>
> Having different direction given invocation is confusing.
>
> Case one (this exact patch):
> ----------------------------
>
> We have different direction between
>
> * hg revert -i
>
> and
>
> * hg revert -i -r any

Again, if we consider the working directory as the reference to compare
to, the direction is not reversed in either case. We'll always be
prompted to accept changes (be they a "discard" or an "apply" operation)
from either the parent (first case, diff hunks as outputted by `hg diff
-r .`) or this "any" revision (second case, diff hunks as outputted by
`hg diff -r . -r any`) to the working directory.

>
> I expect some confusion out of that and as pointed in the previous
> section I think the behavior for '--rev background' is the wrong one.
>
> Case two (keep current state for case point in the first section)
> -----------------------------------------------------------------
>
> We have different direction between
>
> * `hg revert -i`
> * `hg revert -i -r ancestor`
> * `hg revert -i -r precursor`
> * `hg revert -i -r background` #combination of the above,
>
> and
>
> * `hg revert -i -r other`

Unless I missed something, directions should be the same for all cases
where -r is specified (i.e. `hg diff -r . -r REV`).


>> I'm still strongly inclined to take this patch, because you're the
>> only one opposed.
>
> This is a backward compatibility breaking change, confusing some users,
> and with currently no way to restore the previous experience. I greatly
> advice we move with caution here, especially since we have a softer way
> to more forward on the topic.

Which "softer way"? We could not drop the experimental option to let the
users you refer to have the direction "reversed", I guess.