Re: [PATCH 2/3] git-gui: Add the ability to revert selected lines

[Pratyush Yadav <me@yadavpratyush.com>]

+Cc Bert. This has the suggestion I was talking about in one of my 
previous replies.

On 23/08/19 09:28AM, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Maybe the direction taken by this discussion merely suggests that the
> > design is a bit unfortunate. Why "revert"? Why not "stash" instead? Then
> > you don't need to have that annoying confirmation dialog.
> 
> Interesting, but it would need a bit more tweak than a simple
> "stash" for it to be truly helpful, I would imagine.
> 
> The primary purpose of stashing from end user's point of view is to
> save some changes, that are not immediately usable in the context of
> the corrent work, away, so that they can be retrieved later, with a
> side effect of wiping the stashed changes from the working tree. The
> command could be (re|ab)used to wipe local changes you have in the
> working tree, but that would accumulate cruft that you most of the
> time (unless you make a mistake) wanted to discard and wanted to
> never look at again in the stash. If done using the same 'stash' ref
> that is used by the normal "git stash", the stash entries created by
> "git stash" because the user really wanted to keep them for later
> use would be drowned among these "kept just in case" stash entries
> that were created as a side effect of (ab)using stash in place of
> "restore".

Thank you. I was just about to write exactly this.

> But "git stash" can be told to use a different ref, so perhaps by
> using a separate one for this "just in case" purpose, with the
> expectation that the entries are almost always safe to discard
> unless the user made a mistake and take it back immediately
> (i.e. "undo"), it would not hurt the normal use of "git stash" *and*
> give the "revert" necessary safety valve at the same time.

I propose a simpler solution. Essentially how the revert works is it 
generates a diff and stores that in a variable. It then calls git-apply 
with --reverse passed in case of reverts and unstages, and without the 
--reverse in case of staging, and then feeds git-apply the generated 
diff.

So how about we keep a copy of the diff in another variable. This allows 
us to enable undoing of reverts. The obvious limitations are that 
firstly, unless we use a stack/deque that means only one undo is 
allowed. I'm not sure if using an undo stack would be worth the added 
complexity. Secondly, if the working tree is changed between the revert 
and the undo, there are chances of a merge conflict.

If people are okay with these limitations, we can be rid of the 
confirmation dialog while providing a safety net. Sounds like a good 
idea?

-- 
Regards,
Pratyush Yadav