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

[Patrick Steinhardt <ps@pks.im>]

On Fri, Feb 28, 2025 at 09:44:24AM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> >> > 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?

Yup :) I'll adapt then, thanks for chiming in!

Patrick