stash --dwim safety (was Re: What's cooking in git.git (Aug 2009, #06; Sun, 30))

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> [Stalled]
>
> * js/stash-dwim (2009-07-27) 1 commit.
> * tr/reset-checkout-patch (2009-08-27) 9 commits.
>
> There was a discussion on better DWIMmery for the above two topics to (1)
> forbid "git stash save --anything-with-dash" and (2) redirect with any
> option "git stash --opt" to "git stash save --opt", to keep it flexible
> and safe at the same time.  I think it is a sane thing to do, but nothing
> has happened lately.

Actually, I was at fault giving up on Matthieu's patch without studying it
after seeing the phrase "this series replaces", when the two topics the
series tried to replace were already in 'next'.

It turns out that the rework was simple enough, so I did it myself.  Among
his 3 patch series, an equivalent to the first one ("save -keep" can be
written as "save -k" for brevity) were already in, and the second one
(default to "save" if we see any option before command word) was unsafe
without the third one (reject unknown option to "save"), so it ended up as
a single patch that is a combination of the latter two patches.

This applies on top of tr/reset-checkout-patch branch, 14c674e (Make test
case number unique, 2009-08-27).

-- >8 --
From: Matthieu Moy <Matthieu.Moy@xxxxxxx>
Date: Tue, 18 Aug 2009 23:38:40 +0200
Subject: [PATCH] stash: simplify defaulting to "save" and reject unknown options

With the earlier DWIM patches, certain combination of options defaulted
to the "save" command correctly while certain equally valid combination
did not.  For example, "git stash -k" were Ok but "git stash -q -k" did
not work.

This makes the logic of defaulting to "save" much simpler. If the first
argument begins with a '-', it is clear that there is no command word,
and we default to "save" subcommand.

This also teaches "git stash save" to reject an unknown option.  This is
to keep a mistyped "git stash save --quite" from creating a stash with a
message "--quite", and this safety is more important with the new logic
to default to "save" with any option-looking argument without an explicit
comand word.

[jc: this is based on Matthieu's 3-patch series, and he takes all the
credit; if I have introduced bugs while reworking they are mine]

Signed-off-by: Matthieu Moy <Matthieu.Moy@xxxxxxx>
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>

---
 Documentation/git-stash.txt |    1 -
 git-stash.sh                |   22 ++++++++++++++++++----
 t/t3903-stash.sh            |   11 +++++++++++
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 1c4ed41..5d4cce3 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -14,7 +14,6 @@ SYNOPSIS
 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
 'git stash' branch <branchname> [<stash>]
 'git stash' [save [--patch] [-k|--[no-]keep-index] [-q|--quiet] [<message>]]
-'git stash' [-p|--patch|-k|--keep-index]
 'git stash' clear
 'git stash' create
 
diff --git a/git-stash.sh b/git-stash.sh
index 9fd7289..ff71507 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -8,7 +8,6 @@ USAGE="list [<options>]
    or: $dashless ( pop | apply ) [--index] [-q|--quiet] [<stash>]
    or: $dashless branch <branchname> [<stash>]
    or: $dashless [save [-k|--keep-index] [-q|--quiet] [<message>]]
-   or: $dashless [-k|--keep-index]
    or: $dashless clear"
 
 SUBDIRECTORY_OK=Yes
@@ -146,6 +145,14 @@ save_stash () {
 		-q|--quiet)
 			GIT_QUIET=t
 			;;
+		--)
+			shift
+			break
+			;;
+		-*)
+			echo "error: unknown option for 'stash save': $1"
+			usage
+			;;
 		*)
 			break
 			;;
@@ -355,6 +362,13 @@ apply_to_branch () {
 	drop_stash $stash
 }
 
+# The default command is "save"
+case "$1" in
+-*)
+	set "save" "$@"
+	;;
+esac
+
 # Main command set
 case "$1" in
 list)
@@ -406,9 +420,9 @@ branch)
 	apply_to_branch "$@"
 	;;
 *)
-	case $#,"$1","$2" in
-	0,,|1,-k,|1,--keep-index,|1,-p,|1,--patch,|2,-p,--no-keep-index|2,--patch,--no-keep-index)
-		save_stash "$@" &&
+	case $# in
+	0)
+		save_stash &&
 		say '(To restore them type "git stash apply")'
 		;;
 	*)
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index e16ad93..5514f74 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -208,4 +208,15 @@ test_expect_success 'stash -k' '
 	test bar,bar4 = $(cat file),$(cat file2)
 '
 
+test_expect_success 'stash --invalid-option' '
+	echo bar5 > file &&
+	echo bar6 > file2 &&
+	git add file2 &&
+	test_must_fail git stash --invalid-option &&
+	test_must_fail git stash save --invalid-option &&
+	test bar5,bar6 = $(cat file),$(cat file2) &&
+	git stash -- -message-starting-with-dash &&
+	test bar,bar2 = $(cat file),$(cat file2)
+'
+
 test_done
-- 
1.6.4.2.295.g9bcb

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]