Re: [PATCH v3 4/4] for-each-ref: avoid filtering on empty pattern
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Feb 6, 2024, 13:55 UTC
- Message-ID
- <92ba680d-0b48-49f0-aafc-f503e5a5e0ea@gmail.com>
- In-Reply-To
- <CAOLa=ZR=_tt=ppphGMkxqj_YB5G+YkTMWGzRzcHTbrZz4ysb5w@mail.gmail.com>
Hi Karthik
On 06/02/2024 08:52, Karthik Nayak wrote:
Show 21 quoted lines
> 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/
Show 31 quoted lines
>>> 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!