git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Rast <tr@thomasrast.ch>
To: "Øystein Walle" <oystwa@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] stash: handle specifying stashes with spaces
Date: Fri, 29 Nov 2013 20:54:11 +0100	[thread overview]
Message-ID: <87k3frdlwc.fsf@thomasrast.ch> (raw)
In-Reply-To: <1385752979-28162-1-git-send-email-oystwa@gmail.com> ("Øystein Walle"'s message of "Fri, 29 Nov 2013 20:22:59 +0100")

Ø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

  reply	other threads:[~2013-11-29 19:54 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-29 19:22 [PATCH] stash: handle specifying stashes with spaces Øystein Walle
2013-11-29 19:54 ` Thomas Rast [this message]
2013-11-29 21:00   ` [PATCH v2] " Øystein Walle
2013-11-29 21:33     ` Thomas Rast
2013-11-30 15:29       ` Øystein Walle
2013-12-01  0:41 ` [PATCH] " Eric Sunshine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87k3frdlwc.fsf@thomasrast.ch \
    --to=tr@thomasrast.ch \
    --cc=git@vger.kernel.org \
    --cc=oystwa@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).