Re: [PATCH 16/18] fsck: support demoting errors to warnings

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 23, 2014, 18:09 UTC
Message-ID: <xmqqsig62q78.fsf@gitster.dls.corp.google.com>
In-Reply-To: <xmqqwq5i2qtl.fsf@gitster.dls.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 35 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> Okay, so just to clarify: you want me to
>>
>> - split the parser into
>>
>> 	- a parser that accepts only camelCased variable names when they
>> 	  come from the config (for use in fsck and receive-pack), and
>
> OK.
>
>> 	- another parser that rejects camelCased variable names and only
>> 	  accepts lower-case-dashed, intended for command-line parsing
>> 	  in fsck, index-pack and unpack-objects, and
>>
>> - consequently have a converter from the camelCased variable names we
>>   receive from the config in receive-pack so we can pass lower-case-dashed
>>   settings to index-pack and unpack-objects.
>
> I am not sure about the latter two.  This needs a design discussion
> what the command line options should be.
>
> I think the command line should be like this:
>
> 	git cmd --warn=missingTags,missingAuthor
>
> in the first place, i.e. "we may invent tokens to denote new kinds
> of errors as we improve fsck", not with "we may add options for new
> kinds of errors", i.e. the command line should not look like this:
>
> 	git cmd --missing-tags=warn --missing-author=warn
>
> And from that point of view, I see no reason to support the dashed
> variant anywhere in the code, neither in the config parser or in the
> command line parser.

Having said that, I think "missingTags" etc. should not be configuration variable names (instead, they should be values).

Because of that, I do not think we need consistency between the way these "tokens that denote kinds of errors fsck denotes" are spelled and the way "configuration variable names" are spelled.

In other words, I do not think there is nothing that comes from how our configuration variable names are spelled that gives preference to one over the other between the two styles:

(1) Tokens are camelCased

	[fsck]
		warn = missingTagger,missingAuthor
                error = zeroPadTreeEntry

	$ git cmd --warn=missingTagger,missingAuthor

(2) Tokens are dashed-multi-words

	[fsck]
		warn = missing-tagger,missing-author
                error = zero-pad-tree-entry

	$ git cmd --warn=missing-tagger,missing-author

Do I have a strong preference between these two?

Not really. My gut reaction is that (2) may be easier to read, but I can be persuaded either way.

Who else has/had opinions on this? Earlier you said that the configuration the other way, i.e. "[fsck] warn = missingTag", was shot down---who did shoot it? Perhaps that person may be able to point out where in my thinking above I am going in the wrong direction.

Thanks.

[Footnote]

In either case, I'd recommend that we take [ ,]+ as inter-token separator to ease the use on the command line and config file, to allow these:

	[fsck]
		warn = missingTagger missingAuthor
		warn = missingTagger,missingAuthor

	$ git cmd --warn missingTagger,missingAuthor
	$ git cmd --warn "missingTagger missingAuthor"