All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Øystein Walle" <oystwa@gmail.com>
Cc: git@vger.kernel.org, Thomas Rast <tr@thomasrast.ch>
Subject: Re: [PATCH v3] stash: handle specifying stashes with spaces
Date: Tue, 07 Jan 2014 10:42:39 -0800	[thread overview]
Message-ID: <xmqq7gabeiqo.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1389082935-16159-1-git-send-email-oystwa@gmail.com> ("Øystein Walle"'s message of "Tue, 7 Jan 2014 09:22:15 +0100")

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

> When trying to pop/apply a stash specified with an argument containing
> spaces git-stash will throw 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. Make use of rev-parse's --sq option to quote the arguments
> for us to ensure a correct count. Add quotes where necessary.
>
> Also add a test that verifies correct behaviour.
>
> Helped-by: Thomas Rast <tr@thomasrast.ch>
> Signed-off-by: Øystein Walle <oystwa@gmail.com>
> ---
> v3 uses the same eval/--sq technique as v2, suggested by Thomas Rast.
> This is basically a resend except that I added a missing '&&' in the
> test that Eric Sunshine noticed when reading v1.

The change looks good.

An unrelated tangent, but here is a tip-of-the-day for the
approximate parser.  You could have just said

	git stash pop stash@{2.hours}

and it would have been interpreted just the same ;-)

> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index debda7a..7eb011c 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -673,4 +673,15 @@ test_expect_success 'store updates stash ref and reflog' '
>  	grep quux bazzy
>  '
>  
> +test_expect_success 'handle stash specification with spaces' '
> +	git stash clear &&
> +	echo pig > file &&

Style: no SP between redirection operator and its target, i.e.

	echo pig >file &&

> +	git stash &&
> +	test_tick &&
> +	echo cow > file &&
> +	git stash &&
> +	git stash apply "stash@{Thu Apr 7 15:17:13 2005 -0700}" &&

This is brittle.  If new tests are added before this, the test_tick
will give you different timestamp and this test will start failing.

Perhaps grab the timestamp out of the stash that was created, e.g.

	...
        test_tick &&
        git stash &&
        stamp=$(git log -g --format="%cd" -1 refs/stash) &&
	test_tick &&
        echo cow >file &&
        git stash &&
        git stash apply "stash@{$stamp}" &&

or something?

> +	grep pig file
> +'
> +
>  test_done

  reply	other threads:[~2014-01-07 18:42 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-07  8:22 [PATCH v3] stash: handle specifying stashes with spaces Øystein Walle
2014-01-07 18:42 ` Junio C Hamano [this message]
2014-01-07 22:17   ` Thomas Rast
2014-01-07 22:29     ` Junio C Hamano
2014-01-08  0:03       ` Øystein Walle
2014-01-09 18:42         ` Junio C Hamano

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=xmqq7gabeiqo.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=oystwa@gmail.com \
    --cc=tr@thomasrast.ch \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.