Re: [PATCH v4 4/7] stash: introduce new format create

[Thomas Gummerer]

On 02/14, Matthieu Moy wrote:
> Thomas Gummerer  writes:
> 
> >  create_stash () {
> > -   stash_msg="$1"
> > -   untracked="$2"
> > +   stash_msg=
> > +   untracked=
> > +   new_style=
> > +   while test $# != 0
> > +   do
> > +   case "$1" in
> > +   -m|--message)
> > +   shift
> > +   test -z ${1+x} && usage
> > +   stash_msg="$1"
> > +   new_style=t
> > +   ;;
> > +   -u|--include-untracked)
> > +   shift
> > +   test -z ${1+x} && usage
> > +   untracked="$1"
> > +   new_style=t
> > +   ;;
> > +   *)
> > +   if test -n "$new_style"
> > +   then
> > +   echo "invalid argument"
> > +   option="$1"
> > +   # TRANSLATORS: $option is an invalid option, 
> > like
> > +   # `--blah-blah'. The 7 spaces at the beginning 
> > of the
> > +   # second line correspond to "error: ". So you 
> > should line
> > +   # up the second line with however many 
> > characters the
> > +   # translation of "error: " takes in your 
> > language. E.g. in
> > +   # English this is:
> > +   #
> > +   #$ git stash save --blah-blah 2>&1 | head 
> > -n 2
> > +   #error: unknown option for 'stash save': 
> > --blah-blah
> > +   #   To provide a message, use git stash 
> > save -- '--blah-blah'
> > +   eval_gettextln "error: unknown option for 
> > 'stash create': \$option"
> 
> The TRANSLATORS: hint seems a typoed cut-and-paste from somewhere else.
> There are no 7 spaces in this message.
> 
> Actually, if I read the code correctly, $option is not even necessarily
> an option as you're matching *. Perhaps you meant something like
> 
>   -*)
>   option="$1"
>   # TRANSLATORS: $option is an invalid option, like
>   # `--blah-blah'. The 7 spaces at the beginning of the
>   # second line correspond to "error: ". So you should line
>   # up the second line with however many characters the
>   # translation of "error: " takes in your language. E.g. in
>   # English this is:
>   #
>   #$ git stash save --blah-blah 2>&1 | head -n 2
>   #error: unknown option for 'stash save': --blah-blah
>   #   To provide a message, use git stash save -- 
> '--blah-blah'
>   eval_gettextln "error: unknown option for 'stash save': \$option
>To provide a message, use git stash save -- '\$option'"
> usage
> ;;
> *)
>   if test -n "$new_style"
>   then
>   arg="$1"
>   eval_gettextln "error: invalid argument for 'stash 
> create': \$arg"
>   usage
>   fi
> break
>   ;;
> 
> (untested)
> 
> Also, you may want to guard against
> 
>   git stash create "some message" -m "some other message"
> 
> since you are already rejecting
> 
>   git stash create -m "some message" "some other message"
> 
> ? Or perhaps apply "last one wins" for both "-m message" and
> "message"-without-dash-m.

Thanks, you're right I was missing some cases here.  As I just
indicated in [1] however I think we can just make this an internal
interface, instead of user interface facing, so I think we'll need
less error checking.

> -- 
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/

[1]: http://public-inbox.org/git/20170214213038.GE652@hank/

[PATCH v4 4/7] stash: introduce new format create

[Thomas Gummerer]

git stash create currently supports a positional argument for adding a
message.  This is not quite in line with how git commands usually take
comments (using a -m flag).

Add a new syntax for adding a message to git stash create using a -m
flag.  This is with the goal of deprecating the old style git stash
create with positional arguments.

This also adds a -u argument, for untracked files.  This is already used
internally as another positional argument, but can now be used from the
command line.

This introduces a slight regression, when git stash create -m works is
used.  Before this change, it created a stash with the message
"-m works", but now it creates a stash with the message "-m".

Signed-off-by: Thomas Gummerer 
---
 Documentation/git-stash.txt |  1 +
 git-stash.sh| 52 +
 t/t3903-stash.sh|  9 
 3 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 2e9e344cd7..a138ed6a24 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -17,6 +17,7 @@ SYNOPSIS
 [-u|--include-untracked] [-a|--all] []]
 'git stash' clear
 'git stash' create []
+'git stash' create [-m ] [-u|--include-untracked ]
 'git stash' store [-m|--message ] [-q|--quiet] 
 
 DESCRIPTION
diff --git a/git-stash.sh b/git-stash.sh
index 8365ebba2a..6d629fc43f 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -58,8 +58,52 @@ clear_stash () {
 }
 
 create_stash () {
-   stash_msg="$1"
-   untracked="$2"
+   stash_msg=
+   untracked=
+   new_style=
+   while test $# != 0
+   do
+   case "$1" in
+   -m|--message)
+   shift
+   test -z ${1+x} && usage
+   stash_msg="$1"
+   new_style=t
+   ;;
+   -u|--include-untracked)
+   shift
+   test -z ${1+x} && usage
+   untracked="$1"
+   new_style=t
+   ;;
+   *)
+   if test -n "$new_style"
+   then
+   echo "invalid argument"
+   option="$1"
+   # TRANSLATORS: $option is an invalid option, 
like
+   # `--blah-blah'. The 7 spaces at the beginning 
of the
+   # second line correspond to "error: ". So you 
should line
+   # up the second line with however many 
characters the
+   # translation of "error: " takes in your 
language. E.g. in
+   # English this is:
+   #
+   #$ git stash save --blah-blah 2>&1 | head 
-n 2
+   #error: unknown option for 'stash save': 
--blah-blah
+   #   To provide a message, use git stash 
save -- '--blah-blah'
+   eval_gettextln "error: unknown option for 
'stash create': \$option"
+   usage
+   fi
+   break
+   ;;
+   esac
+   shift
+   done
+
+   if test -z "$new_style"
+   then
+   stash_msg="$*"
+   fi
 
git update-index -q --refresh
if no_changes
@@ -268,7 +312,7 @@ push_stash () {
git reflog exists $ref_stash ||
clear_stash || die "$(gettext "Cannot initialize stash")"
 
-   create_stash "$stash_msg" $untracked
+   create_stash -m "$stash_msg" -u "$untracked"
store_stash -m "$stash_msg" -q $w_commit ||
die "$(gettext "Cannot save the current status")"
say "$(eval_gettext "Saved working directory and index state 
\$stash_msg")"
@@ -667,7 +711,7 @@ clear)
;;
 create)
shift
-   create_stash "$*" && echo "$w_commit"
+   create_stash "$@" && echo "$w_commit"
;;
 store)
shift
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index ffe3549ea5..812d0f7a40 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -802,4 +802,13 @@ test_expect_success 'create with multiple arguments for 
the message' '
test_cmp expect actual
 '
 
+test_expect_success 'new style stash create stores correct message' '
+   >foo &&
+   git add foo &&
+   STASH_ID=$(git stash create -m "create test message new style") &&
+   echo "On master: create test message new style" >expect &&
+   git show --pretty=%s -s ${STASH_ID} >actual &&
+   test_cmp expect actual
+'
+
 test_done
-- 
2.11.0.301.g86e6ecc671.dirty