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

[Thomas Gummerer <t.gummerer@gmail.com>]

On 02/14, Matthieu Moy wrote:
> Thomas Gummerer <t.gummerer@gmail.com> 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/