Re: [PATCH] stash: handle specifying stashes with spaces

From: Thomas Rast <tr@thomasrast.ch>
Date: Nov 29, 2013, 19:54 UTC
Message-ID: <87k3frdlwc.fsf@thomasrast.ch>
In-Reply-To: <1385752979-28162-1-git-send-email-oystwa@gmail.com>

Øystein Walle <oystwa@gmail.com> writes:

Show 9 quoted lines

> When trying to pop/apply a stash specified with an argument containing
> spaces the user will see an error:
>
>     $ git stash pop 'stash@{two hours ago}'
>     Too many revisions specified: stash@{two hours ago}
>
> This happens because word splitting is used to count non-option
> arguments. Instead shift the positional arguments as the options are
> processed; the number of arguments left is then the number we're after.

[...]

Show 20 quoted lines

>  	for opt
>  	do
>  		case "$opt" in
>  			-q|--quiet)
>  				GIT_QUIET=-t
> +				shift
>  			;;
>  			--index)
>  				INDEX_OPTION=--index
> +				shift
>  			;;
>  			-*)
>  				FLAGS="${FLAGS}${FLAGS:+ }$opt"
> +				shift
>  			;;
>  		esac
>  	done
>  
> -	set -- $REV
> -

But this isn't correct any more, is it? You unconditionally shift off arguments when you see something of the form -*, even if what you shift is not what you're currently looking at.

For example, without this patch:

  $ g stash apply stash@{0} --index
  On branch next
  Your branch is ahead of 'origin/next' by 41 commits.
    (use "git push" to publish your local commits)
  [blah blah]

but with this patch:

  $ g stash apply stash@{0} --index
  --index is not valid reference

Granted, git-stash is extremely inconsistent in its handling of options. For example, 'git stash save foo -k' does _not_ treat -k as an option. If you set out to unify this (not just randomly (un)break one subroutine) I'd be all for it.

-- 
Thomas Rast
tr@thomasrast.ch