Re: [PATCH] log --decorate: do not leak "commit" color into the next item

From: Jeff King <peff@peff.net>
Date: Feb 21, 2015, 06:23 UTC
Message-ID: <20150221062310.GB29665@peff.net>
In-Reply-To: <xmqqwq3cxjwg.fsf@gitster.dls.corp.google.com>

On Fri, Feb 20, 2015 at 03:06:39PM -0800, Junio C Hamano wrote:

Show 8 quoted lines

> -- >8 --
> Subject: config.txt: spell out how certain typed values are written
> 
> Many variables have values that are not arbitrary strings and there
> are ways to spell these values of certain types.  The way to spell
> colors was described in a section for color.branch.<slot> and other
> variables refered to that section, which was technically wrong, but
> was a bit awkward.

Did you mean "not technically" here?

I think this patch is certainly an improvement with respect to the colors. And I like that it organizes the types into one place using a list, which is easier to scan when you are looking at a manpage. But...

Show 15 quoted lines

> +Values
> +~~~~~~
> +
> +Values of many variables are treated as a simple string, but there
> +are variables that take values of specific types and there are rules
> +as to how to spell them.
> +
> +boolean::
> +	When a variable is said to take a boolean value, many
> +	synonyms are accepted for 'true' and 'false'.
> +	true;; Boolean true can be spelled as `yes`, `on`, `true`,
> +	    or `1`.  Also, a variable defined without `= <value>`
> +	    is taken as true.
> +	false;; Boolean false can be spelled as `no`, `off`,
> +	    `false`, or `0`.

This information is redundant with what is in the `Syntax` section:

  The values following the equals sign in variable assign are all either
  a string, an integer, or a boolean.  Boolean values may be given as
  yes/no, 1/0, true/false or on/off.  Case is not significant in boolean
  values, when converting value to the canonical form using '--bool'
  type specifier; 'git config' will ensure that the output is "true" or
  "false".

I think that paragraph can go away in favor of what you've written. We should mention case-insensitivity there. And the final sentence about `git-config` should go in the section `--bool` description of `git-config`.

Immediately after that paragraph we discuss string values and quoting. Technically those quoting rules apply to all values (e.g., colors, which are just strings with special meaning), but I it may make sense to put them in a "strong::" bullet here.

> +integer::
> +	The value for many variables that specify various sizes can
> +	be suffixed with `k`, `M`,... to mean "scale the number by
> +	1024", "by 1024x1024", etc.

I had a feeling we only did this for ulong's, but I checked the code and we do indeed handle handle unit sizes everywhere. Is it worth mentioning range limits here? I think since c7c377d83f4 it is probably not a big deal. "git config --int" uses 64-bit integers everywhere. Internally we use "unsigned long" for big things. There are still some uses of "int" internally, but only for things that should obviously be small. And in any case we complain if there is overflow. So probably it is not something users need to think about.

-Peff