Re: [PATCH v3 4/4] for-each-ref: avoid filtering on empty pattern

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Karthik

On 06/02/2024 08:52, Karthik Nayak wrote:
> On Mon, Feb 5, 2024 at 7:48 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>> On 29/01/2024 11:35, Karthik Nayak wrote:
>>> When the user uses an empty string pattern (""), we don't match any refs
>>> in git-for-each-ref(1). This is because in git-for-each-ref(1), we use
>>> path based matching and an empty string doesn't match any path.
>>>
>>> In this commit we change this behavior by making empty string pattern
>>> match all references. This is done by introducing a new flag
>>> `FILTER_REFS_NO_FILTER` in `ref-filter.c`, which uses the newly
>>> introduced `refs_for_each_all_refs()` function to iterate over all the
>>> refs in the repository.
>>
>> It actually iterates over all the refs in the current worktree, not all
>> the refs in the repository. I have to say that I find it extremely
>> unintuitive that "" behaves differently to not giving a pattern. I
>> wonder if we can find a better UI here - perhaps a command line option
>> to include pseudorefs?
>>
> 
> As Patrick mentioned, this was discussed a while ago and we decided to
> move forward with the `git for-each-ref ""` syntax.

Thanks I'd missed that discussion. I see that at the end of that 
discussion Junio was concerned that the proposed "" did not account for 
"refs/worktrees/$worktree/*" [1] - how has that been resolved?

Best Wishes

Phillip


[1] https://lore.kernel.org/git/xmqq1qawr6p4.fsf@gitster.g/

>>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>>> ---
>>>    Documentation/git-for-each-ref.txt |  3 ++-
>>>    builtin/for-each-ref.c             | 21 +++++++++++++++++-
>>>    ref-filter.c                       | 13 ++++++++++--
>>>    ref-filter.h                       |  4 +++-
>>>    t/t6302-for-each-ref-filter.sh     | 34 ++++++++++++++++++++++++++++++
>>>    5 files changed, 70 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
>>> index be9543f684..b1cb482bf5 100644
>>> --- a/Documentation/git-for-each-ref.txt
>>> +++ b/Documentation/git-for-each-ref.txt
>>> @@ -32,7 +32,8 @@ OPTIONS
>>>        If one or more patterns are given, only refs are shown that
>>>        match against at least one pattern, either using fnmatch(3) or
>>>        literally, in the latter case matching completely or from the
>>> -     beginning up to a slash.
>>> +     beginning up to a slash. If an empty string is provided all refs
>>> +     are printed, including HEAD and pseudorefs.
>>
>> I think it would be helpful to clarify that it is all the refs for the
>> current worktree that are printed, not all the refs in the repository -
>> we still don't have a way to iterate over the per-worktree refs from
>> other worktrees
>>
> 
> I agree, based on if we keep the current commits or remove them, I'll
> send in a new patch or amend the current series.
> 
> Thanks!