On Tue, Jan 18 2022, Teng Long wrote: > On Fri, Jan 14, 2022 at 7:59 PM Ævar Arnfjörð Bjarmason > <avarab@xxxxxxxxx> wrote: > >> Yes, you get the functionality you need with a simple alias of >> --format='%(objectname)' to --object-name (or whatever), so the only >> reason to carry the extra code is for optimization. >> >> I wonder if the extra difference in performance is still something you >> care about, or if just the --format implementation would be OK. >> >> But in any case, starting with a simpler implementation and testing it >> makes the progression easier to reason about. > > Actually, at first, I wanted to achieve this in a simple way, as the > "--object-only" implementation. > > With the discussion in the community, I think both of them can achieve > this purpose. "--object-only" is more intuitive, while "--format "is > more flexible. > For example, if the terminal supports automatic completion, the function of > this option can be clearly known with typing TAB and lower costs of use and > understanding. "--format" also works, but maybe have to check the help > document to see if there are fields that support the same purpose. > > Because the community had a different opinion about it. Junio, might prefer > an "--object-only" approach, if I understand the context correctly. > > So I have some inclination to support both. However, I can accept that only > "--format" is supported. I'm only talking about how it's implemented internally, not whether we have an --object-only option in the UI. I think it's good to have the option for completion etc. I.e. in my RFC implementation of it here it's just a trivial wrapper around specifying a --format: https://lore.kernel.org/git/RFC-patch-7.7-5e34df4f8dd-20211217T131635Z-avarab@xxxxxxxxx/; Implementing it is 6 lines of trivial C code boilerplate. But when you picked that up & ran with it you ended up carrying your original implementation: https://lore.kernel.org/git/e0274f079a7d381b9a936bfcd53bad64167c18b8.1641440700.git.dyroneteng@xxxxxxxxx/ I'm not saying we shouldn't have that, but that in any case a sequence of: 1. Add a --format option 2. Add a --object-only alias for a --format (what my RFC 7/7 does) 3. Add a custom more optimized --object-only implementation Would make the patch progression much easier to read, and we'd consider the correctness of --object-only (1 and 2) separate from the optimization question (3). But maybe we won't need (3) at all in the end, i.e. is (1 and 2) fast enough for it not to matter (I think probably "yes", but I don't have a strong opinion on that). > So in the next patch, I hope to do some refactoring of the commit to support > "--object-only" as the top commit. If in the end, we decide that "--format" is > enough, we can discard the top "--object-only" commit. *nod*, now that I read ahead I think you pretty much agree with that plan :) > I know you guys currently are busy on the new 2.35 release, so a later reply > is OK. Now would be a good time :) I was reminded of this because Junio's proposed it for next at https://lore.kernel.org/git/xmqqr18jnr2t.fsf@gitster.g/ I think per the above & other replies of mine (including not matters of code arrangement opinion, but e.g. the doc formatting bug) we'll need at least one more re-roll of this. Thanks for sticking with this & working on this! I'll indicate that in a reply to that "What's Cooking" report.
