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