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

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

>> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
>> index 92df596..af11cff 100644
>> --- a/Documentation/git-stash.txt
>> +++ b/Documentation/git-stash.txt
>> @@ -35,11 +35,12 @@ A stash is by default listed as "WIP on
>> 'branchname' ...", but
>>  you can give a more descriptive message on the command line when
>>  you create one.
>>  
>> -The latest stash you created is stored in `refs/stash`; older
>> -stashes are found in the reflog of this reference and can be named using
>> -the usual reflog syntax (e.g. `stash@{0}` is the most recently
>> -created stash, `stash@{1}` is the one before it, `stash@{2.hours.ago}`
>> -is also possible).
>> +The latest stash you created is stored in `refs/stash`; older stashes
>> +are found in the reflog of this reference and can be named using the
>> +usual reflog syntax (e.g. `stash@{0}` is the most recently created
>> +stash, `stash@{1}` is the one before it, `stash@{2.hours.ago}` is also
>> +possible). Stashes may also be references by specifying just the stash
>> +index (e.g. the integer `n` is equivalent to `stash@{n}`).
>
> Yay, a documentation update. Should it be s/references/referenced/ in
> the second-to-last line?

This seems whitespace damaged, though.   I see a few &nbsp; at the
beginning of lines.

Also, Aaron, next time please refrain from reflowing the paragraph
unnecessarily.  I am guessing that you only added one sentence at
the end of an existing paragraph, and such a patch should clearly
show that the only change it did is to append at the end.  Reflowing
will force reviewers to compare the preimage and postimage word by
word to spot what other things were changed.

> So I don't think this is technically a regression in any
> currently-functioning behavior, but it seems like a step in the wrong
> direction to add yet another layer of blind parsing.

Yes.  I agree that the implementation of this patch goes in the
wrong direction, even though it means well.

>> diff --git a/t/t3907-stash-index.sh b/t/t3907-stash-index.sh
>> new file mode 100755
>> index 0000000..72a1838
>> --- /dev/null
>> +++ b/t/t3907-stash-index.sh
>
> Double yay, tests.
>
> Do we really need a whole new script for this, though? There are already
> "stash show" tests in t3903. We should be able to repeat one of them
> using "2" instead of "stash@{2}" (for example).

Yes, it seems a lot better direction to go.  The existing script
t3903 may want to see a bit of modernization clean-up before that to
happen, though.

Thanks for a review.