Re: [PATCH 1/1] config doc: highlight the name=value syntax

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 25, 2018, 22:03 UTC
Message-ID: <xmqqlg7pnskm.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180924222416.5240-2-philipoakley@iee.org>

Philip Oakley <philipoakley@iee.org> writes:

Show 16 quoted lines

> +Variable name/value syntax
> +^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
>  All the other lines (and the remainder of the line after the section
>  header) are recognized as setting variables, in the form
>  'name = value' (or just 'name', which is a short-hand to say that
> @@ -69,7 +72,8 @@ stripped.  Leading whitespaces after 'name =', the remainder of the
>  line after the first comment character '#' or ';', and trailing
>  whitespaces of the line are discarded unless they are enclosed in
>  double quotes.  Internal whitespaces within the value are retained
> -verbatim.
> +verbatim. Single quotes are not special and form part of the
> +variable's value.
>  
>  Inside double quotes, double quote `"` and backslash `\` characters
>  must be escaped: use `\"` for `"` and `\\` for `\`.

Hmph.  This feels a bit backwards.

The original paragraph is horrible in that there is no clear mention that a pair of dq can be used to quote (which primarily is useful if your value have leading or trailing whitespaces); the closest hint is "enclosed in double quotes" we see in the pre-context. The added sentence singles out sq but it is unclear why it is necessary to call out that it is not special---the readers can legitimately wonder if backquotes are special or not and why.

I wonder if this is easier to understand:

    diff --git a/Documentation/config.txt b/Documentation/config.txt
    index ad0f4510c3..5eebd539df 100644
    --- a/Documentation/config.txt
    +++ b/Documentation/config.txt
    @@ -61,12 +61,16 @@ the variable is the boolean "true").
     The variable names are case-insensitive, allow only alphanumeric characters
     and `-`, and must start with an alphabetic character.

    +The value part can have segments that are enclosed in a pair of
    +double quotes (note: other kinds of quoting character pairs are not
    +special)--the double quotes are stripped from the value.
    +
     A line that defines a value can be continued to the next line by
     ending it with a `\`; the backquote and the end-of-line are
     stripped.  Leading whitespaces after 'name =', the remainder of the
     line after the first comment character '#' or ';', and trailing
    -whitespaces of the line are discarded unless they are enclosed in
    -double quotes.  Internal whitespaces within the value are retained
    +whitespaces of the line are discarded.
    +Internal whitespaces within the value are retained
     verbatim.

     Inside double quotes, double quote `"` and backslash `\` characters

Show 7 quoted lines

> @@ -89,10 +93,14 @@ each other with the exception that `includeIf` sections may be ignored
>  if their condition does not evaluate to true; see "Conditional includes"
>  below.
>  
> +Both the `include` and `includeIf` sections implicitly apply an 'if found'
> +condition to the given path names.
> +

Mentioning that missing target file is not an error is definitely an improvement. I've never viewed it as applying "if found" condition myself, but it is not wrong per-se to do so, I would think.

Show 6 quoted lines

>  You can include a config file from another by setting the special
>  `include.path` (or `includeIf.*.path`) variable to the name of the file
>  to be included. The variable takes a pathname as its value, and is
> -subject to tilde expansion. These variables can be given multiple times.
> +subject to tilde expansion and the value syntax detailed above.
> +These variables can be given multiple times.

I have a mild suspicion that this adds negative value. Singling out that "[include] path = ..." follows the usual value syntax makes the readers wonder if there are some "[section] variable = ..." that does not follow the value syntax that they have to be aware of and careful about.