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

From: Junio C Hamano <gitster@pobox.com>
Date: May 13, 2013, 14:34 UTC
Message-ID: <7vfvxrosnf.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1368449154-21882-4-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> 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.

Show 23 quoted lines

> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  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