Re: [PATCH 3/6] stash: simplify option parser for create

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

Re: [PATCH 3/6] stash: simplify option parser for create

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:57:14

Ramkumar Ramachandra [off-list ref] writes:
The option parser for create unnecessarily checks "$1" inside a case
statement that matches "$1" in the first place.  Also, use "$@", not
"$*", as our caller is expecting "$1" "$2", not "$1c$2" (where c is the
first character of IFS).
The first part of the patch may be OK but the rest unfortunately is
wrong.

The semi-user facing "git stash create" never was meant to take
anything but a message sentence and "$*" is the proper way to say
"everything is meant for a single message (just like echo)".
Changing it to "$@" will change the semantics in a big way.
quoted hunk
Signed-off-by: Ramkumar Ramachandra <redacted>
---
 git-stash.sh | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/git-stash.sh b/git-stash.sh
index bbefdf6..0ede313 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -546,11 +546,8 @@ clear)
 	clear_stash "$@"
 	;;
 create)
-	if test $# -gt 0 && test "$1" = create
-	then
-		shift
-	fi
-	create_stash "$*" && echo "$w_commit"
+	shift
+	create_stash "$@" && echo "$w_commit"
 	;;
 drop)
 	shift

Re: [PATCH 3/6] stash: simplify option parser for create

From: Ramkumar Ramachandra <hidden>
Date: 2016-06-15 22:57:14

Junio C Hamano wrote:
The semi-user facing "git stash create" never was meant to take
anything but a message sentence and "$*" is the proper way to say
"everything is meant for a single message (just like echo)".
Changing it to "$@" will change the semantics in a big way.
Ah, I see.  As an interactive caller, it is impossible to set
$untracked (I thought this was a mistake, but you're indicating that
it's intentional).  Okay, I'll fix the patch and documentation.

Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help