Re: git-stash: RFC: Adopt the default behavior to other commands

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: git-stash: RFC: Adopt the default behavior to other commands

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:00

しらいしななこ  [off-list ref] writes:
How about making this behavior configurable?
First, as a general principle, I'd like to avoid having commands that
changes their behaviour drastically depending on who the user is.  It
makes it harder for people experienced a bit more than totally new to
help others.  If they are truly experts and are familiar about the
configuration stash.quick, then they will be fine, but others would say
"Well, it works for me -- 'git stash' itself won't stash but list.  Why
isn't it working for you, I don't know" and scratch head.

Having said that, I reserve rights to change my mind later and start
liking this approach as a compromise.

There are a few suggestions and comments.
+allow_quick_stash () {
+	
+	quick=$(git config stash.quick)
+	if test $? != 0
+	then
I think this is not a per-repository but per-person configuration (I
already said I do not want per-person configuration to affect the
fundamental behaviour of commands, but let's put that objection on hold
for now).  "git config --global" would be more appropriate.

So if the user hasn't seen this behaviour before, then...
+		if ! test -t 0 || ! test -t 1
+		then
+			return 0
+		fi
If it is not interactively called, allow "git stash" sans parameters as
before.  Nice attention to the details.
+		echo '
+*** First time users ***
...
+		git config stash.quick $quick
+		echo '
+You can reconfigure this by editing your $HOME/.gitconfig file'
+
+	fi
Again, you would want --global here.  Also hint about explicit "save"
and "list" in addition to "you can reconfigure" might be helpful.
+	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
Even if it is configured to 'ask', we allow it for non-interactive
session (aka scripts).  Although I would agree with this logic, it could
be debatable.
quoted hunk
@@ -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
I was scratching my head about this extra "echo" and tried your version
after removing it, to realize this is another nice attention to the
details.  Without it, what's output from the command is not very clear
to people who do not know what "git stash" is configured to do for the
session.

Re: git-stash: RFC: Adopt the default behavior to other commands

From: Wincent Colaiuta <hidden>
Date: 2016-06-15 22:44:01

El 20/12/2007, a las 23:31, Junio C Hamano escribió:
しらいしななこ  [off-list ref] writes:
quoted
How about making this behavior configurable?
First, as a general principle, I'd like to avoid having commands that
changes their behaviour drastically depending on who the user is.  It
makes it harder for people experienced a bit more than totally new to
help others.  If they are truly experts and are familiar about the
configuration stash.quick, then they will be fine, but others would  
say
"Well, it works for me -- 'git stash' itself won't stash but list.   
Why
isn't it working for you, I don't know" and scratch head.
Although the patch is well-intentioned, I totally agree with Junio on  
this point.

The solution here isn't configuration, but education. Some people have  
already told how they've been burnt by doing an accidental stash, but  
how many times does this have to happen to you before you learn your  
lesson? Once? Twice if you are very unlucky?

And this is not a very painful lesson to learn, seeing as "git stash"  
is not an inherently destructive operation. In 99% of cases there is  
no risk of hard-to-reverse "damage". If you accidentally stash, you  
can just unstash with "git stash apply". I already posted a two-line  
patch which tells the user how to do this:

http://marc.info/?l=git&m=119799257404542&w=2

The 1% of cases in which "git stash apply" won't work is where the  
user has unsaved changes in running editors at the time they do the  
accidental stash. IMO, this is no justification to change the  
behaviour of stash. Exactly the same is true of other commands that  
alter the working tree; for example, what happens if you use "git  
checkout" to switch to another branch when you have unsaved changes in  
running editors? Are people suggesting that we should change the  
behaviour of "git checkout" to warn the user that they should save any  
unsaved changes before continuing and then hit "y"? I think that such  
a thing would be absurd.

Cheers,
Wincent

Re: git-stash: RFC: Adopt the default behavior to other commands

From: しらいしななこ <hidden>
Date: 2016-06-15 22:44:01

Quoting Junio C Hamano [off-list ref]:
しらいしななこ  [off-list ref] writes:
quoted
How about making this behavior configurable?
First, as a general principle, I'd like to avoid having commands that
changes their behaviour drastically depending on who the user is.  It
makes it harder for people experienced a bit more than totally new to
help others.  If they are truly experts and are familiar about the
configuration stash.quick, then they will be fine, but others would say
"Well, it works for me -- 'git stash' itself won't stash but list.  Why
isn't it working for you, I don't know" and scratch head.
I see.  I usually am not the person who helps but am the 
person who is helped in such a situation, and did not 
consider this issue.
Having said that, I reserve rights to change my mind later and start
liking this approach as a compromise.
I will change the patch as you suggested and resubmit.  Let's 
see if I can change your mind (^_^).

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

----------------------------------------------------------------------
Free pop3 email with a spam filter.
http://www.bluebottle.com/tag/5
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help