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

From: Junio C Hamano <[email-masked]>
Date: Tue, 23 Dec 2014 10:09:31 -0800

Junio C Hamano <[email-masked]> writes:

> Johannes Schindelin <[email-masked]> 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"