From: Junio C Hamano <gitster@pobox.com>
To: "Øystein Walle" <oystwa@gmail.com>
Cc: "Jeff King" <peff@peff.net>,
"Aaron M Watson" <watsona4@gmail.com>, Git <git@vger.kernel.org>,
"Jon Seymour" <jon.seymour@gmail.com>,
"David Caldwell" <david@porkrind.org>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"David Aguilar" <davvid@gmail.com>,
"Alex Henrie" <alexhenrie24@gmail.com>
Subject: Re: [PATCH] stash: allow ref of a stash by index
Date: Wed, 07 Sep 2016 10:50:09 -0700 [thread overview]
Message-ID: <xmqqbmzzoazy.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <CAFaJEqu-JUcwLjrQBk_huSa3DZfCf8O4eAZ=UgcXHzN=CLgtpw@mail.gmail.com> ("Øystein Walle"'s message of "Mon, 5 Sep 2016 23:46:34 +0200")
Øystein Walle <oystwa@gmail.com> writes:
> diff --git a/git-stash.sh b/git-stash.sh
> index 826af18..b026288 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -384,7 +384,7 @@ parse_flags_and_rev()
> i_tree=
> u_tree=
>
> - REV=$(git rev-parse --no-flags --symbolic --sq "$@") || exit 1
> + REV=$(git rev-parse --no-flags --symbolic --sq "$@" 2>/dev/null)
>
> FLAGS=
> for opt
> @@ -422,6 +422,15 @@ parse_flags_and_rev()
> ;;
> esac
>
> + case "$1" in
> + *[!0-9]*)
> + :
OK, so you ignore anything that has a non-digit here, to ensure that...
> + ;;
> + *)
> + set -- "${ref_stash}@{$1}"
... this one triggers only for a string $1 that consists solely of
digits.
> + ;;
> + esac
Makes sense. I notice that both of the two existing case/esac
statements in this function indent case arms and their bodies one
level too deep, which you follwed with the above addition. That may
be something we would want to fix in a follow-up patch.
> REV=$(git rev-parse --symbolic --verify --quiet "$1") || {
> reference="$1"
> die "$(eval_gettext "\$reference is not a valid reference")"
I agree with Peff that the change in error message he noticed is
probably an improvement ;-)
Want to do a final log message to make it a real patch?
Thanks.
prev parent reply other threads:[~2016-09-07 17:50 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-03 23:21 [PATCH] stash: allow ref of a stash by index Aaron M Watson
2016-09-04 1:52 ` Jeff King
2016-09-04 7:01 ` Junio C Hamano
2016-09-04 7:21 ` Johannes Schindelin
2016-09-04 10:57 ` Philip Oakley
2016-09-05 21:46 ` Øystein Walle
2016-09-05 21:58 ` Øystein Walle
2016-09-05 23:52 ` Jeff King
2016-09-07 17:50 ` Junio C Hamano [this message]
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=xmqqbmzzoazy.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=alexhenrie24@gmail.com \
--cc=avarab@gmail.com \
--cc=david@porkrind.org \
--cc=davvid@gmail.com \
--cc=git@vger.kernel.org \
--cc=jon.seymour@gmail.com \
--cc=oystwa@gmail.com \
--cc=peff@peff.net \
--cc=watsona4@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 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.