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

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

On 01/28/2015 11:33 PM, Junio C Hamano wrote:
> We may want to say something about command line option names in the
> new section as well, but for now, let's make sure everybody is clear
> on how to structure and name their configuration variables.
> 
> The text for the rules are partly taken from the log message of
> Jonathan's 6b3020a2 (add: introduce add.ignoreerrors synonym for
> add.ignore-errors, 2010-12-01).
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/CodingGuidelines | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> index 894546d..8fbac15 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -413,6 +413,31 @@ Error Messages
>   - Say what the error is first ("cannot open %s", not "%s: cannot open")
>  
>  
> +Externally Visible Names
> +
> + - For configuration variable names, follow the existing convention:
> +
> +   . The section name indicates the affected subsystem.
> +
> +   . The subsection name, if any, indicates which of an unbounded set
> +     of things to set the value for.
> +
> +   . The variable name describes the effect of tweaking this knob.
> +
> +   The section and variable names that consist of multiple words are
> +   formed by concatenating the words without punctuations (e.g. `-`),
> +   and are broken using bumpyCaps in documentation as a hint to the
> +   reader.
> +
> +   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"? Any set of items that
needs to be programmed one by one is not unbounded in the same sense. It
is true that it can grow over time, but there is a practical limit on
how many such options we would ever implement, and at any given time the
set has a well-defined, finite number of members.

I suppose that this is a reaction to Johannes's proposal [1] to add
configuration settings like

    git config fsck.badDate ignore

[2] which you didn't like [3] but I did [4]. (Did you miss [4], in which
I think I made some good arguments for Johannes's proposal? I don't
think you responded to it.)

I think it would be more productive to finish the concrete discussion
about the "fsck" proposal, and to use the insight gained in that
specific case to inform the decision about whether we need the new
general rule to cover "large fixed set[s] defined by the system".

> + [...] Use
> +   subsection names or variable values, like existing variables
> +   branch.<name>.description and core.whitespace do, instead.

But there is also a precedent for the opposite approach: "advice.*".

    [advice]
            pushUpdateRejected = true|false
            pushNonFFCurrent   = true|false
            statusHints        = true|false
            resolveConflict    = true|false

etc.

In fact, I would argue that

    [core]
            whitespace = blank-at-eol -indent-with-non-tab

is a bad model to follow, and would better have been expressed in a
style like

    [whitespace]
            blankAtEOL = warn
            indentWithNonTab = ignore

for the same reasons that I argued for "fsck.*". The latter style would
also have made it easier to add other choices like
"whitespace.spaceBeforeTab = error". Not that I am advocating that
particular enhancement; I am just giving an example of how the current
style of configuration is less flexible.

> [...]

Michael

[1] http://article.gmane.org/gmane.comp.version-control.git/261068
[2] Actually, the original proposal was for "fsck.bad-date", but I think
we agree that "fsck.badDate" would be the correct version of this proposal.
[3] http://article.gmane.org/gmane.comp.version-control.git/261738
[4] http://article.gmane.org/gmane.comp.version-control.git/262841

-- 
Michael Haggerty
mhagger@alum.mit.edu