Re: [PATCH] stash: allow ref of a stash by index

From: Philip Oakley <philipoakley@iee.org>
Date: Sep 4, 2016, 10:57 UTC
Message-ID: <B8553589D87B4CFAB94CFB22027B9E7D@PhilipOakley>
In-Reply-To: <alpine.DEB.2.20.1609040914200.129229@virtualbox>

From: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>

Show 21 quoted lines

> Hi,
>
> On Sat, 3 Sep 2016, Jeff King wrote:
>
>> On Sat, Sep 03, 2016 at 07:21:18PM -0400, Aaron M Watson wrote:
>>
>> > Allows stashes to be referenced by index only. Instead of referencing
>> > "stash@{n}" explicitly, it can simply be referenced as "n".
>>
>> This says "what" but not "why". I assume it is "because the former is
>> more annoying to type".
>>
>> Are there any backwards-compatibility issues you can think of?
>>
>> I think that "123456" could be a sha1, but I do not see much point in
>> referencing a sha1 as the argument of "stash show". And it looks like
>> this code path is called only from is_stash_like(), so presumably the
>> same logic would apply to other callers.
>
> Maybe we could make it unambiguous, e.g. by using #<n> instead: #123456
> cannot refer to a SHA-1.

The alternative is to limit the length to less that the shortest ambiguous sha1 length that has been used (by Git users - 5, 6, 7? )? Which is probably allowing 1-4 characters, which is a reasonbly deep stash index...

If you need to refer to stash@{9362} you have bigger problems.

Show 8 quoted lines

> But then, '#' are comment-starting in shells, so they would have to by
> escaped. Maybe the best option would be to introduce a -n <n> option,
> with the shortcut -<n> thanks to e0319ff (parseopt: add
> OPT_NUMBER_CALLBACK, 2009-05-07).
>
> Ciao,
> Johannes
>