From: Thomas Rast <tr@thomasrast.ch>
Date: Fri, 29 Nov 2013 19:54:11 GMT
Subject: Re: [PATCH] stash: handle specifying stashes with spaces
Message-ID: <87k3frdlwc.fsf@thomasrast.ch>
In-Reply-To: <1385752979-28162-1-git-send-email-oystwa@gmail.com>

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

> 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.
[...]
>  	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