All of lore.kernel.org
 help / color / mirror / Atom feed
From: Victoria Dye <vdye@github.com>
To: Patrick Steinhardt <ps@pks.im>,
	Victoria Dye via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 8/9] for-each-ref: add option to fully dereference tags
Date: Tue, 7 Nov 2023 17:13:52 -0800	[thread overview]
Message-ID: <cf691b7c-288f-4cc9-a2ac-1a43972ae446@github.com> (raw)
In-Reply-To: <ZUoWWo7IEKsiSx-C@tanuki>

Patrick Steinhardt wrote:
> On Tue, Nov 07, 2023 at 01:26:00AM +0000, Victoria Dye via GitGitGadget wrote:
>> From: Victoria Dye <vdye@github.com>
>>
>> Add a boolean flag '--full-deref' that, when enabled, fills '%(*fieldname)'
>> format fields using the fully peeled target of tag objects, rather than the
>> immediate target.
>>
>> In other builtins ('rev-parse', 'show-ref'), "dereferencing" tags typically
>> means peeling them down to their non-tag target. Unlike these commands,
>> 'for-each-ref' dereferences only one "level" of tags in '*' format fields
>> (like "%(*objectname)"). For most annotated tags, one level of dereferencing
>> is enough, since most tags point to commits or trees. However, nested tags
>> (annotated tags whose target is another annotated tag) dereferenced once
>> will point to their target tag, different a full peel to e.g. a commit.
>>
>> Currently, if a user wants to filter & format refs and include information
>> about the fully dereferenced tag, they can do so with something like
>> 'cat-file --batch-check':
>>
>>     git for-each-ref --format="%(objectname)^{} %(refname)" <pattern> |
>>         git cat-file --batch-check="%(objectname) %(rest)"
>>
>> But the combination of commands is inefficient. So, to improve the
>> efficiency of this use case, add a '--full-deref' option that causes
>> 'for-each-ref' to fully dereference tags when formatting with '*' fields.
> 
> I do wonder whether it would make sense to introduce this feature in the
> form of a separate field prefix, as you also mentioned in your cover
> letter. It would buy the user more flexibility, but the question is
> whether such flexibility would really ever be needed.
> 
> The only thing I could really think of where it might make sense is to
> distinguish tags that peel to a commit immediately from ones that don't.
> That feels rather esoteric to me and doesn't seem to be of much use. But
> regardless of whether or not we can see the usefulness now, if this
> wouldn't be significantly more complex I wonder whether it would make
> more sense to use a new field prefix instead anyway.
> 
> In any case, I think it would be helpful if this was discussed in the
> commit message.
I've been going back and forth on this, but I think a field specifier might
be the way to go after all. Using a field specifier would inherently be more
complex than the command line option (since the formatting code is a bit
complicated), but that's not an insurmountable problem. The thing I kept
getting caught up on was which symbol (or symbols?) to use to indicate a full
object peel. I mentioned `**fieldname` in the cover letter, but that looks
more like a double dereference than a recursive one.

I think `^{}fieldname` would be a good candidate, but it's *extremely*
important (for the sake of avoiding user confusion/frustration) that it
produces the same object & associated info as the standard revision parsing
machinery [1]. One notable difference (it might be the only one) from
`*fieldname` would be, if a ref points to a non-tag object, then that
object's information would printed (rather than an empty string). But maybe
that difference is what we'd want anyway, since it's a better one-for-one
replacement of 'git for-each-ref | git cat-file --batch-check'.

I'll try implementing that for V2. If it doesn't work for some reason,
though, I'll explain why in the commit message.

[1] https://git-scm.com/docs/git-rev-parse#Documentation/git-rev-parse.txt-emltrevgtemegemv0998em

> 
> Patrick
> 

  reply	other threads:[~2023-11-08  1:13 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-07  1:25 [PATCH 0/9] for-each-ref optimizations & usability improvements Victoria Dye via GitGitGadget
2023-11-07  1:25 ` [PATCH 1/9] ref-filter.c: really don't sort when using --no-sort Victoria Dye via GitGitGadget
2023-11-07 10:49   ` Patrick Steinhardt
2023-11-07 18:13     ` Victoria Dye
2023-11-07  1:25 ` [PATCH 2/9] for-each-ref: clarify interaction of --omit-empty & --count Victoria Dye via GitGitGadget
2023-11-07 19:23   ` Øystein Walle
2023-11-07 19:30     ` Victoria Dye
2023-11-08  7:53       ` Øystein Walle
2023-11-08 10:00         ` Kristoffer Haugsbakk
2023-11-07  1:25 ` [PATCH 3/9] ref-filter.h: add max_count and omit_empty to ref_format Victoria Dye via GitGitGadget
2023-11-07  1:25 ` [PATCH 4/9] ref-filter.h: move contains caches into filter Victoria Dye via GitGitGadget
2023-11-07 10:49   ` Patrick Steinhardt
2023-11-07  1:25 ` [PATCH 5/9] ref-filter.h: add functions for filter/format & format-only Victoria Dye via GitGitGadget
2023-11-07  1:25 ` [PATCH 6/9] ref-filter.c: refactor to create common helper functions Victoria Dye via GitGitGadget
2023-11-07 10:49   ` Patrick Steinhardt
2023-11-07 18:41     ` Victoria Dye
2023-11-07  1:25 ` [PATCH 7/9] ref-filter.c: filter & format refs in the same callback Victoria Dye via GitGitGadget
2023-11-07 10:49   ` Patrick Steinhardt
2023-11-07 19:45     ` Victoria Dye
2023-11-07  1:26 ` [PATCH 8/9] for-each-ref: add option to fully dereference tags Victoria Dye via GitGitGadget
2023-11-07 10:50   ` Patrick Steinhardt
2023-11-08  1:13     ` Victoria Dye [this message]
2023-11-08  3:14       ` Junio C Hamano
2023-11-08  7:19         ` Patrick Steinhardt
2023-11-08 18:02         ` Victoria Dye
2023-11-09  1:22           ` Junio C Hamano
2023-11-09  1:23           ` Junio C Hamano
2023-11-09  1:32             ` Junio C Hamano
2023-11-07  1:26 ` [PATCH 9/9] t/perf: add perf tests for for-each-ref Victoria Dye via GitGitGadget
2023-11-07  2:36 ` [PATCH 0/9] for-each-ref optimizations & usability improvements Junio C Hamano
2023-11-07  2:48   ` Victoria Dye
2023-11-07  3:04     ` Junio C Hamano
2023-11-07 10:49     ` Patrick Steinhardt
2023-11-08  1:31       ` Victoria Dye
2023-11-14 19:53 ` [PATCH v2 00/10] " Victoria Dye via GitGitGadget
2023-11-14 19:53   ` [PATCH v2 01/10] ref-filter.c: really don't sort when using --no-sort Victoria Dye via GitGitGadget
2023-11-16  5:29     ` Junio C Hamano
2023-11-14 19:53   ` [PATCH v2 02/10] ref-filter.h: add max_count and omit_empty to ref_format Victoria Dye via GitGitGadget
2023-11-16 12:06     ` Øystein Walle
2023-11-14 19:53   ` [PATCH v2 03/10] ref-filter.h: move contains caches into filter Victoria Dye via GitGitGadget
2023-11-14 19:53   ` [PATCH v2 04/10] ref-filter.h: add functions for filter/format & format-only Victoria Dye via GitGitGadget
2023-11-16  5:39     ` Junio C Hamano
2023-11-14 19:53   ` [PATCH v2 05/10] ref-filter.c: rename 'ref_filter_handler()' to 'filter_one()' Victoria Dye via GitGitGadget
2023-11-14 19:53   ` [PATCH v2 06/10] ref-filter.c: refactor to create common helper functions Victoria Dye via GitGitGadget
2023-11-14 19:53   ` [PATCH v2 07/10] ref-filter.c: filter & format refs in the same callback Victoria Dye via GitGitGadget
2023-11-14 19:53   ` [PATCH v2 08/10] for-each-ref: clean up documentation of --format Victoria Dye via GitGitGadget
2023-11-14 19:53   ` [PATCH v2 09/10] ref-filter.c: use peeled tag for '*' format fields Victoria Dye via GitGitGadget
2023-11-16  5:48     ` Junio C Hamano
2023-11-14 19:53   ` [PATCH v2 10/10] t/perf: add perf tests for for-each-ref Victoria Dye via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cf691b7c-288f-4cc9-a2ac-1a43972ae446@github.com \
    --to=vdye@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.