[PATCH] Make "git stash" configurable

[PATCH] Make "git stash" configurable

[しらいしななこ]

"git stash" without argument originally created an unnamed
stash, but some people felt this can be confusing to new
users.  This introduces per-user config variable stash.quick to
control this behavior.

The variable can take one of three values: true, false, ask.

When set to "true", the command allows to create a quick
stash without any user interaction.  When set to "false",
the command shows the list of stash instead.  When set to
"ask", the command asks the user.

For the first time users, when the variable is not set,
the command helps the user to set it interactively.

Signed-off-by: Nanako Shiraishi <nanako3@bluebottle.com>
---
 Documentation/git-stash.txt |   17 +++++++++-
 git-stash.sh                |   72 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 85 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index c0147b9..5d39da2 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -22,11 +22,24 @@ and reverts the working directory to match the `HEAD` commit.
 The modifications stashed away by this command can be listed with
 `git-stash list`, inspected with `git-stash show`, and restored
 (potentially on top of a different commit) with `git-stash apply`.
-Calling git-stash without any arguments is equivalent to `git-stash
-save`.  A stash is by default listed as "WIP on 'branchname' ...", but
+A stash is by default listed as "WIP on 'branchname' ...", but
 you can give a more descriptive message on the command line when
 you create one.
 
+Calling git-stash without any arguments is equivalent to `git-stash
+save` when the command is run non-interactively.  When running
+interactively, the value of the `stash.quick` configuration
+variable in `$HOME/.gitconfig` determines what happens.
+
+ - If `stash.quick` is set to `true`, a stash without message is
+   created.
+
+ - If `stash.quick` is set to `false`, it shows the list of
+   stash, without creating a new one.
+
+ - If `stash.quick` is set to `ask`, the user is asked which
+   behavior is desired.
+
 The latest stash you created is stored in `$GIT_DIR/refs/stash`; 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
diff --git a/git-stash.sh b/git-stash.sh
index f16fd9c..b6223bd 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -192,6 +192,69 @@ apply_stash () {
 	fi
 }
 
+allow_quick_stash () {
+
+	quick=$(git config --global stash.quick)
+	if test $? != 0
+	then
+		if ! test -t 0 || ! test -t 1
+		then
+			return 0
+		fi
+
+		echo '
+*** First time users ***
+
+"git stash" can create an unnamed stash entry without user interaction.
+This is a quick way to save away your work in progress.  Some people
+find this behaviour confusing or dangerous to new users.  You can
+configure the command to list the existing stash entries instead.'
+
+		while :
+		do
+			echo '
+Do you want the command without argument to always...
+
+1. Ask for confirmation
+2. Create an unnamed stash
+3. List existing stash entries
+'
+			printf 'Which one? [1/2/3] '
+			read reply
+			quick=
+			case "$reply" in
+			1|A*)	quick=ask ;;
+			2|C*)	quick=true ;;
+			3|L*)	quick=false ;;
+			*)	continue ;;
+			esac
+			break
+		done
+		git config --global stash.quick $quick
+		echo '
+You can reconfigure this by editing your $HOME/.gitconfig file'
+
+	fi
+
+	case "$quick" in
+	true)	return 0 ;;
+	false)	return 1 ;;
+	ask)	: do not return ;;
+	esac
+
+	if ! test -t 0 || ! test -t 1
+	then
+		return 0
+	fi
+
+	printf 'Do you want to create an unnamed stash? [Y/n] '
+	read reply
+	case "$reply" in
+	[nN]*)	return 1 ;;
+	*)	return 0 ;;
+	esac
+}
+
 # Main command set
 case "$1" in
 list)
@@ -226,11 +289,16 @@ create)
 	create_stash "$*" && echo "$w_commit"
 	;;
 *)
-	if test $# -eq 0
+	if test $# -ne 0
+	then
+		usage
+	fi
+	if allow_quick_stash
 	then
 		save_stash && git-reset --hard
 	else
-		usage
+		echo "*** Stash List ***"
+		list_stash
 	fi
 	;;
 esac
-- 
1.5.3.7

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

----------------------------------------------------------------------
Free pop3 email with a spam filter.
http://www.bluebottle.com/tag/5

Re: [PATCH] Make "git stash" configurable

[Steven Grimm]

On Dec 21, 2007, at 2:22 AM, しらいしななこ wrote:

> "git stash" without argument originally created an unnamed
> stash, but some people felt this can be confusing to new
> users.  This introduces per-user config variable stash.quick to
> control this behavior.

This whole thing smells of indecision to me. We should figure out a  
behavior we want and go with it. Git's command line interface is  
already confusing enough without adding this kind of variability --  
any new user is going to look at this and say, "Why couldn't they just  
make up their minds?"

My vote is to require "save". We already have an alias mechanism that  
people can use to turn "git stash save" into a two-word command. And I  
say this as someone who runs "git stash" and will have to retrain his  
fingers.

-Steve (who thinks "trying to please everyone at all times" is usually  
a source of bad UI design decisions)

Re: [PATCH] Make "git stash" configurable

[Matthieu Moy]

Steven Grimm <koreth@midwinter.com> writes:

> Git's command line interface is already confusing enough without
> adding this kind of variability

It also makes portable scripts harder. It's easy to write a script
that works only if you have the option set ...

-- 
Matthieu

Re: [PATCH] Make "git stash" configurable

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> Steven Grimm <koreth@midwinter.com> writes:
>
>> Git's command line interface is already confusing enough without
>> adding this kind of variability
>
> It also makes portable scripts harder. It's easy to write a script
> that works only if you have the option set ...

In general I'd agree but in this particular case I do not think
that applies.  Scripts can always say what they want, either
"stash save" or "stash list".

Re: [PATCH] Make "git stash" configurable

[Steven Grimm]

On Dec 21, 2007, at 9:54 AM, Junio C Hamano wrote:
> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>> It also makes portable scripts harder. It's easy to write a script
>> that works only if you have the option set ...
>
> In general I'd agree but in this particular case I do not think
> that applies.  Scripts can always say what they want, either
> "stash save" or "stash list".

I bet Matthieu meant that it's easy for someone who runs bare "git  
stash" all the time to forget that there's an option to make that  
command do something different, and to thus code up a local script  
that runs the command as they're used to running it. Such a script  
will work just fine for them but it'll break as soon as they give it  
to someone else who has the option set differently.

-Steve

Re: [PATCH] Make "git stash" configurable

[Junio C Hamano]

Steven Grimm <koreth@midwinter.com> writes:

> On Dec 21, 2007, at 2:22 AM, しらいしななこ wrote:
>
>> "git stash" without argument originally created an unnamed
>> stash, but some people felt this can be confusing to new
>> users.  This introduces per-user config variable stash.quick to
>> control this behavior.
>
> This whole thing smells of indecision to me.

If you want to force me to decide, you know what the decision it
would be.  Here are the guiding principles and decisions based
on them.

 (1) A tool should be optimized for people who know what it does
     and how to use it.

     While forbidding "git stash" without argument and forcing
     people who want to have a quick stash to type "git stash
     save" may appear not to be adding too many keystrokes, it
     actually is a regression.  Because "git stash save" without
     arguments make a much less descriptive stash than the
     current quickie "git stash", doing so slows them down by
     forcing them to think of what message to give.

     The decision would be to keep "git stash" quickie stash
     creation.  An alternative decision could be to do nothing
     when "git stash" is given, and make "git stash save" (and
     "git stash create") without the actual message to come up
     with a better default message.

 (2) A tool should not change its behaviour too drastically
     depending on who the user is.

     Otherwise it makes it harder for people with mid-level
     experience to teach less experienced people.  The decision
     would be that "stash.quick=ask" and "stash.quick=no" are
     unacceptable.

 (3) A tool should support safety for a mistaken use by who know
     the tool (even they can make mistakes).

     I listed this mostly because we made "git clean" to violate
     principle (2) above.  Even when you know what that command
     does, it is easy to mean "make clean" and type "git clean"
     by mistake.  And "git clean" is very destructive --- there
     is no "oops, I take it back".  This principle has led us to
     introduce clean.requireforce that defaults to true.

     For "git stash", there is no "make stash" or "xyzzy stash"
     that can easily induce mistyping.  Even if it were typed by
     mistake, there is no such destructive "oops, I cannot take
     it back" problem with "git stash", either.  "git stash
     apply" will get you back to where you were.  The decision
     would again be that "stash.quick=no" is unwanted.

 (4) A tool should support safety for clueless people when it is
     reasonable.

     Even though "I did not know what command foo does, so I
     tried running it and it did something unexpected" is a
     silly excuse to rob quickie "git stash" from people who
     know better, we cannot avoid the fact that there are
     clueless people.  I think checking stash.iknowwhatitdoes to
     detect first-time users, and explaining what it does to
     them, may make sense.  And we can take hints from the patch
     that started this thread how to do this.

     The decision here is that I am open to a change that
     implements the one-time safety instruction.

Note that in the above guiding principles, I used "should" in
RFC2119/BCP0014 sense: there may exist valid reasons in
particular circumstances to ignore a particular item, but the
full implications must be understood and carefully weighed
before choosing a different course.

[PATCH] Emit helpful status for accidental "git stash" save

[Wincent Colaiuta]

El 21/12/2007, a las 20:48, Junio C Hamano escribió:

> (4) A tool should support safety for clueless people when it is
>    reasonable.
>
>    Even though "I did not know what command foo does, so I
>    tried running it and it did something unexpected" is a
>    silly excuse to rob quickie "git stash" from people who
>    know better, we cannot avoid the fact that there are
>    clueless people.  I think checking stash.iknowwhatitdoes to
>    detect first-time users, and explaining what it does to
>    them, may make sense.  And we can take hints from the patch
>    that started this thread how to do this.
>
>    The decision here is that I am open to a change that
>    implements the one-time safety instruction.

Here's a less invasive change that I think addresses this point (4)
that you make.

-- 8< --
Emit helpful status for accidental "git stash" save

If the user types "git stash" mistakenly thinking that this will list
their stashes he/she may be surprised to see that it actually saved
a new stash and reset their working tree and index.

In the worst case they might not know how to recover the state. So
help them by telling them exactly what was saved and also how to
restore it immediately.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
 git-stash.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index f16fd9c..a2f3723 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -99,7 +99,8 @@ save_stash () {
 
 	git update-ref -m "$stash_msg" $ref_stash $w_commit ||
 		die "Cannot save the current status"
-	printf >&2 'Saved "%s"\n' "$stash_msg"
+	printf >&2 'Saved working directory and index state "%s"\n' "$stash_msg"
+	echo >&2 '(To restore them type "git stash apply")'
 }
 
 have_stash () {
-- 
1.5.4.rc0.68.g15eb8-dirty

Re: [PATCH] Emit helpful status for accidental "git stash" save

[Junio C Hamano]

Wincent Colaiuta <win@wincent.com> writes:

> diff --git a/git-stash.sh b/git-stash.sh
> index f16fd9c..a2f3723 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -99,7 +99,8 @@ save_stash () {
>  
>  	git update-ref -m "$stash_msg" $ref_stash $w_commit ||
>  		die "Cannot save the current status"
> -	printf >&2 'Saved "%s"\n' "$stash_msg"
> +	printf >&2 'Saved working directory and index state "%s"\n' "$stash_msg"
> +	echo >&2 '(To restore them type "git stash apply")'
>  }
>  
>  have_stash () {

I like that "To restore them..." insn, and I like that this is
much less invasive than anything we have seen during the
discussion.  But can we do this only for an accidental "git
stash" not for a deliberate "git stash save"?

[PATCH v2] Emit helpful status for accidental "git stash" save

[Wincent Colaiuta]

El 22/12/2007, a las 17:22, Junio C Hamano escribió:

> I like that "To restore them..." insn, and I like that this is
> much less invasive than anything we have seen during the
> discussion.  But can we do this only for an accidental "git
> stash" not for a deliberate "git stash save"?

Something like this then?

-- 8< --
Emit helpful status for accidental "git stash" save

If the user types "git stash" mistakenly thinking that this will list
their stashes he/she may be surprised to see that it actually saved
a new stash and reset their working tree and index.

In the worst case they might not know how to recover the state. So
help them by telling them exactly what was saved and also how to
restore it immediately.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
 git-stash.sh |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index f16fd9c..ab52b6f 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -13,6 +13,7 @@ TMP="$GIT_DIR/.git-stash.$$"
 trap 'rm -f "$TMP-*"' 0
 
 ref_stash=refs/stash
+ARGC=$#
 
 no_changes () {
 	git diff-index --quiet --cached HEAD -- &&
@@ -99,7 +100,9 @@ save_stash () {
 
 	git update-ref -m "$stash_msg" $ref_stash $w_commit ||
 		die "Cannot save the current status"
-	printf >&2 'Saved "%s"\n' "$stash_msg"
+	printf >&2 'Saved working directory and index state "%s"\n' "$stash_msg"
+	test $ARGC -eq 0 &&
+		echo >&2 '(To restore them type "git stash apply")'
 }
 
 have_stash () {
-- 
1.5.4.rc0.68.g15eb8-dirty

Re: [PATCH v2] Emit helpful status for accidental "git stash" save

[Junio C Hamano]

Wincent Colaiuta <win@wincent.com> writes:

> El 22/12/2007, a las 17:22, Junio C Hamano escribió:
>
>> I like that "To restore them..." insn, and I like that this is
>> much less invasive than anything we have seen during the
>> discussion.  But can we do this only for an accidental "git
>> stash" not for a deliberate "git stash save"?
>
> Something like this then?

Sure, even like this.

 git-stash.sh |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index f16fd9c..ade300c 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -99,7 +99,7 @@ save_stash () {
 
 	git update-ref -m "$stash_msg" $ref_stash $w_commit ||
 		die "Cannot save the current status"
-	printf >&2 'Saved "%s"\n' "$stash_msg"
+	printf >&2 'Saved working directory and index state "%s"\n' "$stash_msg"
 }
 
 have_stash () {
@@ -228,7 +228,9 @@ create)
 *)
 	if test $# -eq 0
 	then
-		save_stash && git-reset --hard
+		save_stash && 
+		echo >&2 '(To restore them type "git stash apply")' &&
+		git-reset --hard
 	else
 		usage
 	fi

Re: [PATCH v2] Emit helpful status for accidental "git stash" save

[Wincent Colaiuta]

El 22/12/2007, a las 18:58, Junio C Hamano escribió:

> Wincent Colaiuta <win@wincent.com> writes:
>
>> Something like this then?
>
> Sure, even like this.
>
> git-stash.sh |    6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/git-stash.sh b/git-stash.sh
> index f16fd9c..ade300c 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -99,7 +99,7 @@ save_stash () {
>
> 	git update-ref -m "$stash_msg" $ref_stash $w_commit ||
> 		die "Cannot save the current status"
> -	printf >&2 'Saved "%s"\n' "$stash_msg"
> +	printf >&2 'Saved working directory and index state "%s"\n'  
> "$stash_msg"
> }
>
> have_stash () {
> @@ -228,7 +228,9 @@ create)
> *)
> 	if test $# -eq 0
> 	then
> -		save_stash && git-reset --hard
> +		save_stash &&
> +		echo >&2 '(To restore them type "git stash apply")' &&
> +		git-reset --hard
> 	else
> 		usage
> 	fi

Preserves the behaviour while avoiding the need for the creation of  
the ARGC variable, so if you prefer this solution then it seems fine  
by me.

Cheers,
Wincent