Re: [PATCH 2/9] builtin/cat-file: wire up an option to filter objects

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 28, 2025, 17:44 UTC
Message-ID: <xmqqikou2ccn.fsf@gitster.g>
In-Reply-To: <Z8GVKLee75GDG2S4@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 15 quoted lines

>> > Note that we don't use the same `--filter=` name fo the option as we use
>> > in git-rev-list(1). We already have `--filters`, and having both
>> > `--filter=` and `--filters` would be quite confusing. Instead, the new
>> > option is called `--objects-filter`.
>> 
>> I'm not sure I agree. I would rather have consistency in various
>> commands. Because `--filters` doesn't accept an argument, so I would say
>> having both `--filters` and `--filter=` is fine. I see in various places
>> we already use `OPT_PARSE_LIST_OBJECTS_FILTER` which defines the option
>> as `--filter=`, so it's pretty standard for several commands. I'd
>> prefer git-cat-file(1) to follow that as well. But that's my 2 cents.
>
> I'll wait for a third party to chime in as a tie breaker here :) I'm not
> feeling overly strong about it, but still think that it's just too easy
> to get wrong when those options are so extremely similarly named.

$ git grep '^[^a-z]*--filter' Documentation/ Documentation/config/gc.adoc: `--filter=<filter-spec>` option of linkgit:git-repack[1]. Documentation/git-cat-file.adoc:--filters:: Documentation/git-cat-file.adoc: `--filters`. Documentation/git-clone.adoc: [--filter=<filter-spec>] [--also-filter-submodules]] [--] <repository> Documentation/git-clone.adoc:`--filter=<filter-spec>`:: Documentation/git-clone.adoc: `--filter=blob:limit=<size>` will filter out all blobs of size Documentation/git-pack-objects.adoc:--filter=<filter-spec>:: Documentation/git-repack.adoc:--filter=<filter-spec>:: Documentation/git-repack.adoc:--filter-to=<dir>:: Documentation/rev-list-options.adoc:--filter=<filter-spec>:: Documentation/rev-list-options.adoc:--filter-provided-objects:: Documentation/rev-list-options.adoc:--filter-print-omitted::

The above does makes it look that whoever called their invention "--filters" when they added it to "cat-file" wasn't paying attention to make things consistent, but that is not the case. The word "filter" in the context of existing feature set of "cat-file" has ALWAYS refered to the act of applying the "smudge" filter chain to externalize an internal "clean" blob object contents for the working tree representation. We should thank that somebody for not using and squatting on a shorter and sweeter "--filter" ;-)

"cat-file" should call the feature "--filter=<filter-spec>" like everybody else does, or the feature should not be added to "cat-file" at all. Unless we are willing to rename "--filter=" options for _all_ existing commands to "--object-filter=", that is.

In retrospect, such a longer and more explicit name may have been nicer. But given that all users of the "--filter=<filter-spec>" are about object transfer, it is understandable that we didn't invoke deliberate redundancy when naming the option. Historically, "cat-file" has always been "give me the contents of the object I name", and never about "I may ask about many objects but do not answer requests for objects chosen by these criteria", so it also is understandable that we didn't redundantly say "--contents-filter", too.

Am I third-party enough?