Warning suggestion for git stash drop

Warning suggestion for git stash drop

[Laurent Humblet]

Hi all,

Would it be possible to add an optional Yes/No when doing a 'git stash
drop'?  Something similar as what happens when pushing on a remotely
checked out branch (with a config setting to turn the warning on/off).

I know that you can always get your dropped stash back using git
reflog but a small warning might be a useful feature to avoid unwanted
stash drops on a regular basis.

Thank you for your work on this amazing little software,
All the best,
Laurent

Re: Warning suggestion for git stash drop

[Junio C Hamano]

Laurent Humblet <laurent.humblet@gmail.com> writes:

> Would it be possible to add an optional Yes/No when doing a 'git stash
> drop'?  Something similar as what happens when pushing on a remotely
> checked out branch (with a config setting to turn the warning on/off).
>
> I know that you can always get your dropped stash back using git
> reflog but a small warning might be a useful feature to avoid unwanted
> stash drops on a regular basis.

I sympathize with this, but the same principle also would apply many
destructive commands like "git reset --hard", "git rm <path>", etc.
and also "/bin/rm -f" ;-)

I however do not think a good general way to do this without
breaking people's scripts.  When they do 'stash drop' in their
scripts, they know they want to get rid of the dropped stash
entries, and they expect that the user may not necessarily be
sitting in front of the shell to give the command a Yes (they
probably wouldn't even give the user a message "the next step
may ask you to say Yes; please do so").

On the other hand, just like "git reset --hard" and "git clean -f"
does not have such safety (i.e. the user is aware that the command
is destructive by giving "--hard" and "-f"), "drop" may be a sign
that the user knowingly doing something destructive.

So I dunno.  

Re: Warning suggestion for git stash drop

[Stefan Beller]

On Thu, Jun 29, 2017 at 11:44 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Laurent Humblet <laurent.humblet@gmail.com> writes:
>
>> Would it be possible to add an optional Yes/No when doing a 'git stash
>> drop'?  Something similar as what happens when pushing on a remotely
>> checked out branch (with a config setting to turn the warning on/off).
>>
>> I know that you can always get your dropped stash back using git
>> reflog but a small warning might be a useful feature to avoid unwanted
>> stash drops on a regular basis.
>
> I sympathize with this, but the same principle also would apply many
> destructive commands like "git reset --hard", "git rm <path>", etc.
> and also "/bin/rm -f" ;-)
>
> I however do not think a good general way to do this without
> breaking people's scripts.  When they do 'stash drop' in their
> scripts, they know they want to get rid of the dropped stash
> entries, and they expect that the user may not necessarily be
> sitting in front of the shell to give the command a Yes (they
> probably wouldn't even give the user a message "the next step
> may ask you to say Yes; please do so").
>
> On the other hand, just like "git reset --hard" and "git clean -f"
> does not have such safety (i.e. the user is aware that the command
> is destructive by giving "--hard" and "-f"), "drop" may be a sign
> that the user knowingly doing something destructive.
>
> So I dunno.
>

I was about to propose to have an easy undo operation, such as the
reflog. But then I checked the man page for git-stash and it already says:

       older stashes are found in the reflog of this reference and can be named
       using the usual reflog syntax (e.g. stash@{0} is the most recently
       created stash, stash@{1} is the one before it, stash@{2.hours.ago} is
       also possible). Stashes may also be referenced by specifying
just the stash
       index (e.g. the integer n is equivalent to stash@{n})

Maybe that needs to be polished a bit more?

I myself used the stash back then (I don't use it any more), and I assumed
the stash was a stack of changes, but the data structure seems to imply it
is only one thing that can be stashed and the reflog enables the stacking
part, such that a dropped stash is gone and is not recorded in the reflog.
Dropping a stash seems to just erase the topmost line from the stash reflog,
so it really is as destructive an "/bin/rm -f"

So getting back a dropped stash via the reflog is not via asking for a stash
reflog but rather for HEADs or a branchs reflog, which may be complicated.

Re: Warning suggestion for git stash drop

[Laurent Humblet]

Thank you for your feedback.

I suppose that turning a hypothetical confirmation option 'on' would
impact a stash pop for instance as it automatically drops the stash if
it was applied without conflicts.

What about a --confirm flag?  You could then simply alias 'git stash
drop --confirm' locally and it wouldn't impact anything else?

Have a great week-end!

On 29 June 2017 at 20:56, Stefan Beller <sbeller@google.com> wrote:
> On Thu, Jun 29, 2017 at 11:44 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Laurent Humblet <laurent.humblet@gmail.com> writes:
>>
>>> Would it be possible to add an optional Yes/No when doing a 'git stash
>>> drop'?  Something similar as what happens when pushing on a remotely
>>> checked out branch (with a config setting to turn the warning on/off).
>>>
>>> I know that you can always get your dropped stash back using git
>>> reflog but a small warning might be a useful feature to avoid unwanted
>>> stash drops on a regular basis.
>>
>> I sympathize with this, but the same principle also would apply many
>> destructive commands like "git reset --hard", "git rm <path>", etc.
>> and also "/bin/rm -f" ;-)
>>
>> I however do not think a good general way to do this without
>> breaking people's scripts.  When they do 'stash drop' in their
>> scripts, they know they want to get rid of the dropped stash
>> entries, and they expect that the user may not necessarily be
>> sitting in front of the shell to give the command a Yes (they
>> probably wouldn't even give the user a message "the next step
>> may ask you to say Yes; please do so").
>>
>> On the other hand, just like "git reset --hard" and "git clean -f"
>> does not have such safety (i.e. the user is aware that the command
>> is destructive by giving "--hard" and "-f"), "drop" may be a sign
>> that the user knowingly doing something destructive.
>>
>> So I dunno.
>>
>
> I was about to propose to have an easy undo operation, such as the
> reflog. But then I checked the man page for git-stash and it already says:
>
>        older stashes are found in the reflog of this reference and can be named
>        using the usual reflog syntax (e.g. stash@{0} is the most recently
>        created stash, stash@{1} is the one before it, stash@{2.hours.ago} is
>        also possible). Stashes may also be referenced by specifying
> just the stash
>        index (e.g. the integer n is equivalent to stash@{n})
>
> Maybe that needs to be polished a bit more?
>
> I myself used the stash back then (I don't use it any more), and I assumed
> the stash was a stack of changes, but the data structure seems to imply it
> is only one thing that can be stashed and the reflog enables the stacking
> part, such that a dropped stash is gone and is not recorded in the reflog.
> Dropping a stash seems to just erase the topmost line from the stash reflog,
> so it really is as destructive an "/bin/rm -f"
>
> So getting back a dropped stash via the reflog is not via asking for a stash
> reflog but rather for HEADs or a branchs reflog, which may be complicated.

Re: Warning suggestion for git stash drop

[Junio C Hamano]

Laurent Humblet <laurent.humblet@gmail.com> writes:

> Thank you for your feedback.
>
> I suppose that turning a hypothetical confirmation option 'on' would
> impact a stash pop for instance as it automatically drops the stash if
> it was applied without conflicts.
>
> What about a --confirm flag?  You could then simply alias 'git stash
> drop --confirm' locally and it wouldn't impact anything else?

I think that is probably trivial to add, but how would you make sure
you give it?  One way may be to train your fingers to type "git sd"
with something like this in your ~/.gitconfig:

	[alias] sd = "stash drop --confirm"

but at that point, you could instead have something like the
following in you ~/bin/git-sd and get the same effect:

	#!/bin/sh
	if tty -s
	then
		echo >&2 "are you sure you want to drop all stash entries?"
		case "$(read)" in
		[Yy]*) ;;
		*) echo >&2 "ok, let's not drop 'em"; exit 0 ;;
                esac
	fi
	exec git stash drop

without adding the "--confirm" option at all.

So I am not sure that would get us closer to a satisfactory solution
to your original problem.  

Retroactively adding an end-user safety is hard.

> Have a great week-end!

You too.

Re: Warning suggestion for git stash drop

[Laurent Humblet]

Oh great, an answer 20 days later ... Sorry about this.

I have been reading a bit on the topic and found that for Bash we
should be using shell functions over aliases so I wrote this function
to be simply added to the .bashrc:

# 'git stash drop' confirm shell function
git() {
    if [[ $@ == "stash drop" ]]; then
        read -r -p "Are you sure you want to drop your stash? [y/N] " response
        case "$response" in
            [yY][eE][sS]|[yY])
                command git "$@"
                ;;
        esac
    else
        command git "$@"
    fi
}

Be careful, I'm no Bash expert and I don't believe it is fully POSIX
compliant but it's a good start I think.  This could also be used for
the 'rm -rf' command or any other potentially dangerous command I
believe.  Maybe there is a place for a .bashrc_safe_shell_functions
that could be loaded by the user to add confirmation before all
potentially dangerous commands but it'll be for another thread.

Thank you for your help guys and keep up with your excellent work on Git!

All the best,
Laurent


On 30 June 2017 at 21:21, Junio C Hamano <gitster@pobox.com> wrote:
> Laurent Humblet <laurent.humblet@gmail.com> writes:
>
>> Thank you for your feedback.
>>
>> I suppose that turning a hypothetical confirmation option 'on' would
>> impact a stash pop for instance as it automatically drops the stash if
>> it was applied without conflicts.
>>
>> What about a --confirm flag?  You could then simply alias 'git stash
>> drop --confirm' locally and it wouldn't impact anything else?
>
> I think that is probably trivial to add, but how would you make sure
> you give it?  One way may be to train your fingers to type "git sd"
> with something like this in your ~/.gitconfig:
>
>         [alias] sd = "stash drop --confirm"
>
> but at that point, you could instead have something like the
> following in you ~/bin/git-sd and get the same effect:
>
>         #!/bin/sh
>         if tty -s
>         then
>                 echo >&2 "are you sure you want to drop all stash entries?"
>                 case "$(read)" in
>                 [Yy]*) ;;
>                 *) echo >&2 "ok, let's not drop 'em"; exit 0 ;;
>                 esac
>         fi
>         exec git stash drop
>
> without adding the "--confirm" option at all.
>
> So I am not sure that would get us closer to a satisfactory solution
> to your original problem.
>
> Retroactively adding an end-user safety is hard.
>
>> Have a great week-end!
>
> You too.