Re: [GSoC][RFC PATCH v4 3/5] builtin/refs: add list subcommand

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Aug 1, 2025, 15:49 UTC
Message-ID: <c4d9b25a-2a62-41bf-90e6-455c62b03aa3@gmail.com>
In-Reply-To: <xmqqseibm7ap.fsf@gitster.g>

On 01/08/2025 15:43, Junio C Hamano wrote:

Show 28 quoted lines

> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>> I find the behavior of for-each-ref quite confusing so I wonder if we
>> really want to copy it to the new command. For example
>>
>>      git for-each-ref 'refs/h*'
>>
>> does not print anything but
>>
>>      git for-each-ref 'refs/heads/m*'
>>
>> prints all the branches beginning with m. Another example is
>>
>>      git for-each-ref 'refs/heads'
>>
>> prints all the branches but
>>
>>      git for-each-ref 'refs/heads*'
>>
>> prints nothing. To me it would be much easier to understand if the
>> pattern always required an explicit wildcard (or possible always did a
>> prefix match in the absence of a wildcard).
> 
> Thanks for raising this point.
> 
> One point you are wrong is 'refs/heads/m*'; it does not list all the
> branches that begin with "m".  It will not show "refs/heads/mid/night"
> even though it may show "refs/heads/morning'.

Thanks for correcting that

> I do not think we want to require wildcard (in other words,
> refs/heads/ and refs/heads that result in the prefix matching
> behaviour that is anchored at hierarchy boundary is a good thing,
> and we should not require refs/heads/* to get it).

It is confusing that refs/heads does a prefix match but refs/heads/m does not (unless there is a hierarchy boundary after the m). Maybe we should require a trailing slash to trigger a prefix match?

Show 9 quoted lines

> As the "list the refs with various criteria" feature itself is
> shared with "refs list", it would make the entire system even more
> confusing if their criteria to choose which ones to show are
> different.
> 
> I _think_ the current selection criteria is basically the prefix
> match that is anchored at hierarchy boundary, and a single asterisk
> expands only to a single hierarchy element without crossing
> hierarchy boundary.

Having had a bit of a play with different patterns I think that's right.

Show 5 quoted lines

> It is very handy that refs/heads/* expands to
> the main integration branches (master, next, seen, maint-*) without
> showing pw/3.0-commentchar-auto-deprecation branch and others.
> 
> What does a double-asterisk currently do in these patterns?

refs/heads/m** seems to behave like refs/heads/m*. I'm a bit surprised by that as for-each-ref seems to set WM_PATHNAME and I thought that our wildmatch code used '**' to match any character in that case.

Show 12 quoted lines

> If it
> is not doing anything useful, perhaps we should make it match any
> letter, without getting constrained by hierarchy boundaries?  IOW,
> a "fix" might be to make sure the following happens?
> 
>   - "refs/heads/m*" matches all local branches whose name starts with
>     'm' like 'morning', but not the ones inside subhierarchies that
>     start with 'm' like 'mid/night'.
> 
>   - "refs/heads/m**" matches all local branches whose name starts
>     with 'm' and in the ones inside subhierarchies that start with
>     'm'.

That sounds like a good idea

Thanks

Phillip