Re: [PATCH 4/4] cat-file: support --textconv/--filters in batch mode

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 19, 2016, 16:19 UTC
Message-ID: <xmqqtweg3f4s.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160819150632.wa4wvidzqmrrkcfs@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 9 quoted lines

>> Sorry, Torsten, this is not my doing. So I cannot explain why it is not an
>> enum.
>> 
>> I *guess* the rationale for 'c' being the cmdmode of --textconv is "c for
>> convert". That is why I chose "w as in worktree" as new cmdmode.
>
> I think it started as a 'char' because it was representing the
> single-letter options of "cat-file -e", "cat-file -s", etc. And then the
> textconv patches started the monstrosity of "c".

I think it was merely 't' was taken for "-t" (one beauty of using the OPT_CMDMODE for options with shorthand is that we do not need any enum, as the short-form options already form an enum), so "textConv" was the compromise. It could have been 'T' or even \C-t ;-)

> I don't think cleaning it up needs to be part of your series, but it
> might be nice low-hanging fruit for somebody to clean up on top.

I agree.

I briefly wondered if we want to add a check to ensure uniqueness of these cmdmode letters in parse_options_check(), but that would forbid us from having synonymous options (e.g. "am --continue" and "am --resolved" both map to the same), so it would not work.