Re: [PATCH 1/2] stash: record positional index in 'struct stash_info'

[Ben Knoble <ben.knoble@gmail.com>]

[on mobile, so only looking at patch context]

> Le 30 juil. 2026 à 12:41, Junio C Hamano <gitster@pobox.com> a écrit :
> 
> get_stash_info() resolves revision arguments (such as 'stash@{0}'
> or '2') and checks whether they refer to 'refs/stash', but it
> does not allow callers to determine the 0-based positional
> reflog index.
> 
> Record '.stash_idx' in 'struct stash_info'.  Populate it in
> get_stash_info(), setting it to 0 when omitted (defaulting
> to the latest stash), to 'n' when a valid positional index
> '@{n}' is specified, or to -1 when the index specification
> is invalid or non-positional (such as a time-based reference).
> 
> Subcommands that manipulate reflog entries by index can use
> '.stash_idx' directly, instead of parsing the revision arguments
> themselves.

I notice even after 2/2 we don’t have any users of this index yet (except rejecting invalid entries as the series goal).

> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> builtin/stash.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
> 
> diff --git a/builtin/stash.c b/builtin/stash.c
> index c4809f299a..5041a9ba81 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -175,6 +175,7 @@ struct stash_info {
>   struct strbuf revision;
>   int is_stash_ref;
>   int has_u;
> +    int stash_idx;
> };
> 
> #define STASH_INFO_INIT { \
> @@ -248,6 +249,7 @@ static int get_stash_info(struct stash_info *info, int argc, const char **argv)
>   char *expanded_ref;
>   const char *revision;
>   const char *commit = NULL;
> +    const char *at;
>   struct object_id dummy;
>   struct strbuf symbolic = STRBUF_INIT;
> 
> @@ -300,6 +302,19 @@ static int get_stash_info(struct stash_info *info, int argc, const char **argv)
>   }
> 
>   free(expanded_ref);
> +
> +    at = strstr(revision, "@{");
> +    if (at) {
> +        char *ep;
> +        unsigned long u = strtoul(at + 2, &ep, 10);
> +        if (ep > at + 2 && *ep == '}' && u < 100000000)
> +            info->stash_idx = (int)u;

What’s the purpose of the 1e8 constant/comparison? I see we truncate the unsigned long to an int, but even on 32-bit platforms 1e8 is a small portion of the integer range, right? So my read is that we are limiting the valid « n » in @{n}. I’m not totally sure why, though, or if that matches with the rest of the stash manipulation code.  

> +        else
> +            info->stash_idx = -1;
> +    } else {
> +        info->stash_idx = 0;
> +    }
> +
>   return !(ret == 0 || ret == 1);
> }
> 
> --
> 2.55.0-597-ge6126a35d6