Hi Junio, On Tue, 23 Dec 2014, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes: > > > The parser I wrote actually accepts both versions, allowing me to skip > > the tedious step to convert the camelCased config setting into a > > lower-case-dashed version to pass to `index-pack` or `unpack-objects`, > > only to be parsed by the same parser as `fsck` would use directly. > > > > So I am rather happy with the fact that the parser handles both > > camelCased and lower-case-dashed versions. > > That is myopic view of the world that ignores maintainability and > teachability, doing disservice to our user base. 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 - 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. If you want it this way, I will do it this way. > What message does it send to an unsuspecting new user that > fsck.random-error is silently accepted (because we will never document > it) as an alias for fsck.randomError, while most of the configuration > variables will not take such an alias? I will not participate in a discussion about consistency again. There is nothing that can be done about it; what matters is what you will accept and what not. I will make the code stricter (and consequently more complex) if that is what you want. > >> I suspect that it would be much better if the configuration variables > >> were organized the other way around, e.g. > >> > >> $ git config fsck.warn missingTagger,someOtherKindOfError > > > > I had something similar in an earlier version of my patch series, but > > it was shot down rightfully: if you want to allow inheriting defaults > > from $HOME/.gitconfig, you have to configure the severity levels > > individually. > > Hmmm. What's wrong with "fsck.warn -missingTagger" that overrides > the earlier one, or even "fsck.info missingTagger" after having > "fsck.warn other,missingTagger,yetanother", with the usual "last one > wins" rule? I will change the code (next year...). > >> But the proposed organization to use one variable per questionable > >> event type (as opposed to one variable per severity level) would lead > >> to a one-shot override of this form, e.g. > >> > >> $ git fsck --missing-tagger=warn --some-other-kind-of-error=warn > >> > >> which I think is insane to require us to support unbound number of > >> dashed options. > > > > The intended use case is actually *not* the command-line, but the config > > file, in particular allowing /etc/gitconfig, $HOME/.gitconfig *and* > > .git/config to customize the settings. > > But we do need to worry about one-shot override from the command > line. A configuration that sticks without a way to override is a > no-no. And of course you can, by specifying the config setting via the -c command-line option. The only inconsistency here is that all other command-line options are lower-case-dashed, while the config settings are camelCased. > >> Or are you saying that we allow "git config core.file-mode true" from > >> the command line to set core.fileMode configuration? > > > > I do not understand this reference. > > I was puzzled by your "command line" and wondering if you meant > "from the command line, aVariable can be spelled a-variable". Well, of course, if you call `git -c aVariable command --option=a-variable` you have a nice accumulation of styles right there ;-) > > I did not suggest to change `git config`, did I? If I did, I > > apologize; it was definitely *not* my intention to change > > long-standing customs. > > Then fsck.missing-tagger is definitely out. Long standing customs > is that a multi-word token at the first and the last level is not > dashed-multi-word. But I already changed all of the patches to fsck.missingTagger. The only thing I did not do yet is to split the parser into two, one accepting only camelCased, one accepting only lower-case-dashed options, and a translator to convert from camelCase to lower-case-dashed versions (because it is a lot of work and additional complexity, as well as opportunity for bugs to hide because we'll have three code paths). I asked you above whether you want that, and I will do it if you say that you do. Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html