UI discussions for revert --interactive and uncommit --interactive

Martin von Zweigbergk martinvonz at google.com
Tue May 26 15:25:14 CDT 2015


On Sun, May 24, 2015 at 3:08 PM Pierre-Yves David <
pierre-yves.david at ens-lyon.org> wrote:

>
>
> On 05/22/2015 06:01 PM, Martin von Zweigbergk wrote:
> >
> >
> > On Fri, May 22, 2015 at 3:51 PM Pierre-Yves David
> > <pierre-yves.david at ens-lyon.org <mailto:pierre-yves.david at ens-lyon.org>>
> > wrote:
> >
> >
> >
> >     On 05/22/2015 05:37 PM, Laurent Charignon wrote:
> >      > Hi,
> >      >
> >      > As you may have seen I worked on revert --interactive and soon I
> will
> >      > send some patches for hg uncommit --interactive.
> >      >
> >      > Both these commands raise a UI question that I have been debating
> >     with
> >      > marmoute and sid0.
> >      > The --interactive session let your choose what changes are
> reverted.
> >      > There is two main way to ask the question
> >      > We wanted to have your thoughts on it, so I put a minimal example
> >     below
> >      > with the two propositions.
> >      > For the record, I want *proposition 1* and marmoute wants
> >     *proposition 2*
> >
> >     The way I phrase the two solutions is as follow:
> >
> >     1) Show the action revert will perform (current implementation),
> >     2) Show the change revert will cancel,
> >
> >
> > When reverting to another revision (picking pieces from that other
> > revision), I'm quite sure I want 1). And, therefore, I think I want 1)
> > in all cases for consistency.
>
> Your sentence make me realized we can use revert in two ways.
> a) reverting against an ancestor: (actually revert changes)
> b) reverting against a "descendant": (fetching from it).
>

There are revisions that are neither of those (siblings, uncles and such).


> In the ancestor case (a), `hg diff --rev (a)` would be consistent with
> what solution (2) show during revert.
> In the descendant case (b), `hg diff --rev (b)` would be consistent with
> what solution (1) show during revert.
>
> I've to admit that I barely use the (b) case anymore. I used to need it
> to rework stack of patch, but uncommit/fold/etc are fitting this need
> for me now. My biggest usage of `hg revert` is without a revision to
> clean up silly change (debug print) or against an ancestors to try to
> get something to work again.
>
> The consistency argument is interesting here, let's widen it. The most
> important point to me is the consistency of the experience across all
> commands. Mercurial is a VCS, vcs are mostly about handling "changes"
> (hunks in this context). The vast majority of command present such
> change in the same direction, from parent to child.
>
> commands          | direction
> hg diff           | parent -> child
> hg diff -r (a)    | parent -> child
> hg diff -r (b)    | child  -> parent <- one exception
>

With this history:
x---a---c
     \
      @---b

Currently, it behaves this way (with clean working copy):
hg diff -r a | diff from a to @
hg diff r b  | diff from b to @
hg diff -r c | diff from c to @

I think you're suggesting:
hg diff -r a | diff from a to @
hg diff r b  | diff from @ to b
hg diff -r c | diff from @ to c

I fear that a "hg diff -r a" followed by a "hg diff -r c" would surprise
people, because the hunks corresponding to changes between @ and 'a' would
be reversed between the two invocations.



> hg diff -r .::(b) | parent -> child
> hg commit -i      | parent -> child
> hg amend -i       | parent -> child
> hg export         | parent -> child
>
> So I would argues having the revert inverting (solution 1) this
> direction to be inconsistent. Solution 2 would keep the pattern of
> display consistent:
>
> hg diff             | parent -> child
> hg diff -r (a)      | parent -> child
> hg diff -r (b)      | child  -> parent
> hg revert           | parent -> child
> hg revert -r (a)    | parent -> child
> hg revert -r (b)    | child  -> parent
> hg uncommit         | parent -> child
> hg uncommit -r (a)  | parent -> child
> hg uncommit -r (b)  | child  -> parent
>
>
> This inconsistency is especially problematic because it make the hunk
> looks different and therefor much harder to recognize. This inversion is
> not just about switching the "-" and "+" char around. They implies a
> deep visual changes:
>
> - inversion in the green/red color , especially the size of block:
>    (I want to revert this big green block -> huuu there is just a big
> red block now).
>
> - inversion in the order of line: all additions because deletion and are
> set as first line, changing the shape of the "hunk".
>
> Personally this deep visual changes induce a massive cognitive overhead
> for me.


I do understand your confusion :-(. I was tempted to suggest a new command
for the "pick pieces from other revision" operation, but then a new command
for each of your examples would be needed. I don't really have a good
answer. Sorry.


> With solution (1) every other command I use show me changes in a
> given way but `revert -i` (with curse UI). I cannot rely on pattern
> recognition anymore.
>
>
> If `hg revert` were consistent with the other commands especially `hg
> diff`, I could combine some operation in my workflow. For example, being
> able to review your changes `hg diff` and clean up debug output and
> other silliness `hg revert`. The curse UI is a very powerful tool to
> navigate and review your change. I fire up `hg commit -i` instead of `hg
> diff; hg commit # if all okay` on a regular basis. Having `hg revert -i`
> display the same data let me perform such cleanup the same way.
>
> This way to use the UI can be pushed further. I remember some
> discussions at a sprint about a `hg split -i` UI which would use a
> crecord like UI and let you assign number to chunk (that one is in the
> first commit, that one in the second one), having a "forget about this
> change" (uncommit|revert it) category in such command would fit well,
> and it will have to be "parent" -> child" direction.
>
>
> (yep, I know, this email is getting quite long, but I've a last point to
> make)
>
> I understand this argument of solution (1) is about"showing what is the
> action revert will do". But I think it is a bit to low level. As I said
> before, Mercurial is mostly about managing "changes"
>
> - hg diff -> show your change current uncommited (and other)
> - hg commit -> put your change into a commit
> - hg amend -> add more change to the current commit
> - hg uncommit -> move change from commit to working copu
> - hg revert -> remove current changes (and other)
>
> And so solution (2) focus on showing these changes. The fact that
> "change" makes more sense than "action" become more visible when looking
> at 'uncommit'. `hg uncommit -i` is going to take a "change" a move it
> from the current commit to the working copy. The action, performed on
> the commit is "apply the reverse of that change", but at the same time
> it (kind of) apply the same change back to your working copy (in
> practice, file are untouched, but `hg diff` gained that change). So
> `uncommit` is doing "action"s in both direction and "picking the
> direction the action is done" will not help picking a side here (And we
> want `hg uncommit -i` and `hg revert -i` to be consistent, especially
> since `hg uncommit` may gain a --revert option). So showing "the change
> to be reverted" is a sensible things to do, in particular since the
> command is called `hg revert` not `hg apply reverse-diff-from`.
>
>
> To highlight this "change" centric UI and mitigate the "revert show the
> inverse of the action" concern, the two variants of solution (2) have to
> be looked at.
>
> - The basic approach (2.1): Select change to be reverted.
>    (here is the work you've have done, do you want to throw it away)
> - The alternative approach (2.2):
>    We revert everything, select change to be preserved.
>
> The alternative approach mean different questions and default value,
> with the same core display for the change. This probably get use best of
> both world:
>
> - Diff display match the action performed (preserve X)
> - Diff are not displayed reversed.
>
> (The hg revert -ir (b) get maybe a bit awkward, but it is not the core
> usage of `hg revert`)
>
> To conclude, if we cannot read a simple consensus quickly, I advice for
> having an experimental config options and get each side of the debate to
> test the other approach until 3.5 freeze.
>
> --
> Pierre-Yves David
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150526/dae1bbc9/attachment.html>


More information about the Mercurial-devel mailing list