Re: [PATCH 3/3] CodingGuidelines: describe naming rules for configuration variables

[Michael Haggerty <mhagger@alum.mit.edu>]

Junio,

Thanks for your thoughtful response.

On 02/01/2015 09:18 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> On 01/28/2015 11:33 PM, Junio C Hamano wrote:
>>> +   When choosing the variable namespace, do not use variable name for
>>> +   specifying possibly unbounded set of things, most notably anything
>>> +   an end user can freely come up with (e.g. branch names), but also
>>> +   large fixed set defined by the system that can grow over time
>>> +   (e.g. what kind of common whitespace problems to notice).
>>
>> I think we can all agree with this rule for "anything an end user can
>> freely come up with". Such sets are truly unbounded.
>>
>> But what is the justification for applying it to "large fixed set
>> defined by the system that can grow over time"?
> [...]
> 
> I can see it argued that for things that are completely independent
> (e.g. the consequence of setting fsck.badDate can never be affected
> by how fsck.missingTagger is configured), separate configuration
> variables may not be wrong per-se, but I can see that a set of knobs
> that would have been originally independent, as the operation grow
> richer, gain more semantics to make them related, and at that point,
> I doubt "they are internally independent; expose them as independent
> to the end users" argument would hold water that well.
> 
> A good example is "core.whitespace", where you would have started
> with a simple set of booleans ("blank-at-eol" and "space-before-tab"
> are conceptually independent and will stay to be), but once you
> start supporting "indent-with-non-tab" and "tab-in-indent" you
> cannot pretend that they are independent.
> 
> And that is the "existing practice" I primarily had in mind.  We
> didn't do
> 
> 	whitespace.tabInIndent = true
>         whitespace.indentWithNonTab = true
> 
> to pretend they are independent and still internally having to make
> sure the consistency of the setting.  We structured the syntax for
> ease of the end user (not scripter) to shorter
> 
> 	core.whitespace = tab-in-indent,indent-with-non-tab
> 
> as we need the consistency thing either way (and it is easier to see
> the consistency mistakes when they appear next to each other).
> 
> And I am happy that we chose wisely in an early version that didn't
> use one-variable-per-knob but used list-of-features-as-value instead.

You make an interesting point: values that start as a list of
independent booleans can grow dependencies over time.

In retrospect, ISTM that a better interface for the indentation-related
"whitespace" settings would have been something like

* "whitespace.indent" -- a single value chosen from "tabs-only |
spaces-only | tabs-when-possible | anything"
* "whitespace.tabwidth" -- an integer value

This would have made the mutual-exclusivity of those choices manifest in
the style of configuration rather than hoping that the user notices that
his settings contradict each other.

Let's dig into this example some more by imagining some other
hypothetical future extensions. Suppose we wanted to support different
whitespace rules for different types of files [1]. For example, I might
never want to forbid "cr-at-eol" everywhere, and might usually like to
uses spaces for my indentation, but for Makefiles need to indent using
tabs. The "type 2" syntax, I think, is pretty straightforward:

    [whitespace]
            cr-at-eol = error
            indent = spaces-only

    [whitespace "makefile"]
            indent = tabs-only

Our usual rules, "last setting wins" and "foo.*.bar, if present, takes
precedence overrides foo.bar", make it pretty clear how the above should
be interpreted.

What would be the "type 1" syntax for this? Would "cr-at-eol" be
inherited from "core.whitespace" to "core.makefile.whitespace"? If not,
then I have to repeat "cr-at-eol":

    [core]
            whitespace = cr-at-eol tab-in-indent
    [core "makefile"]
            whitespace = cr-at-eol indent-with-non-tab

[2]. If values are inherited, then do I also have to countermand
"tab-in-indent" in the "makefile" rule?

    [core]
            whitespace = cr-at-eol tab-in-indent
    [core "makefile"]
            whitespace = indent-with-non-tab -tab-in-indent

Or does "indent-with-non-tab" automatically supersede "tab-in-indent",
according to last-setting-wins (an interpretation that starts requiring
the config parser to have domain-specific knowledge)?:

    [core]
            whitespace = cr-at-eol tab-in-indent
    [core "makefile"]
            whitespace = indent-with-non-tab

But if that is the case, which setting wins in this scenario?:

    [core]
            whitespace = cr-at-eol tab-in-indent
    [core "makefile"]
            whitespace = indent-with-non-tab
    # In another config file maybe:
    [core]
            whitespace = space-before-tab

Does "core.whitespace = space-before-tab" supersede
"core.makefile.whitespace = indent-with-non-tab" here?

No matter which of the "type 1" variants we choose, we would have to
invent new rules for users and config parsers to learn, and some of
those rules would require domain-specific knowledge to interpret.
Whereas the "type 2" style is pretty straightforward and leans only on
existing conventions.

> [...]

Michael

[1] I'm not claiming that this specific extension makes sense. It might
make more sense to configure the whitespace rules one-by-one using
gitattributes. But I think it is nevertheless a typical way that
features are extended and therefore an interesting gedankenexperiment.

[2] For the purposes of this discussion, let's ignore the fact that
there is no precedent for a three-level "core" setting like
"core.makefile.whitespace". It could just as easily be spelled
"whitespace.makefile.check" or something in the "type 1" syntax.

-- 
Michael Haggerty
mhagger@alum.mit.edu