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 there are no
non-flag arguments, it is clear that there is no command word, and we
default to "save" subcommand.  This rule prevents "git stash -q apply"
from quietly creating a stash with "apply" as the message.

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 a follow-up
discussion, and he and Peff take all the credit; if I have introduced bugs
while reworking, they are mine.]

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 1c4ed412a12ffe6393aa39499169ad8e96d47fcf..30a249cbc5a5f45df88f9698b192e84b883faefa 100644 (file)
--- 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' ( 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
 
 'git stash' clear
 'git stash' create
 


@@ -46,9 +45,11 @@ OPTIONS
 save [--patch] [--[no-]keep-index] [-q|--quiet] [<message>]::
 
        Save your local modifications to a new 'stash', and run `git reset
 save [--patch] [--[no-]keep-index] [-q|--quiet] [<message>]::
 
        Save your local modifications to a new 'stash', and run `git reset

-       --hard` to revert them.  This is the default action when no
-       subcommand is given. The <message> part is optional and gives
-       the description along with the stashed state.
+       --hard` to revert them.  The <message> part is optional and gives
+       the description along with the stashed state.  For quickly making
+       a snapshot, you can omit _both_ "save" and <message>, but giving
+       only <message> does not trigger this action to prevent a misspelled
+       subcommand from making an unwanted stash.

 +
 If the `--keep-index` option is used, all changes already added to the
 index are left intact.
 +
 If the `--keep-index` option is used, all changes already added to the
 index are left intact.

diff --git a/git-stash.sh b/git-stash.sh
index 9fd72894c49b22174c0c10cb3f83fa57314c74fb..f24337613bab39402d704374aa681d52fc2f2313 100755 (executable)
--- 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 ( 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
    or: $dashless clear"
 
 SUBDIRECTORY_OK=Yes


@@ -146,6 +145,14 @@ save_stash () {
                -q|--quiet)
                        GIT_QUIET=t
                        ;;
                -q|--quiet)
                        GIT_QUIET=t
                        ;;

+               --)
+                       shift
+                       break
+                       ;;
+               -*)
+                       echo "error: unknown option for 'stash save': $1"
+                       usage
+                       ;;

                *)
                        break
                        ;;
                *)
                        break
                        ;;


@@ -355,6 +362,18 @@ apply_to_branch () {
        drop_stash $stash
 }
 
        drop_stash $stash
 }
 

+# The default command is "save" if nothing but options are given
+seen_non_option=
+for opt
+do
+       case "$opt" in
+       -*) ;;
+       *) seen_non_option=t; break ;;
+       esac
+done
+
+test -n "$seen_non_option" || set "save" "$@"
+

 # Main command set
 case "$1" in
 list)
 # Main command set
 case "$1" in
 list)


@@ -406,9 +425,9 @@ branch)
        apply_to_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")'
                ;;
        *)
                say '(To restore them type "git stash apply")'
                ;;
        *)

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index e16ad93d2c2718baf62ad90e9fc07187106d9ee4..5514f74b30aa74fe2bf214e90f9ad8f4da2876e4 100755 (executable)
--- 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 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
 test_done