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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux