[Q] should "color.*.<slot> = normal" emit nothing?

[Q] should "color.*.<slot> = normal" emit nothing?

[Junio C Hamano]

If you wanted to paint the HEAD decoration as the same color as the
body text (primarily because cyan is too faint on a black-on-white
terminal to be readable) you would not want to say

    [color "decorate"]
        head = black

because that you would not be able to reuse same configuration on
a white-on-black terminal.  I would naively expect

    [color "decorate"]
        head = normal

to work, but it does not.  I notice that we have these definitions
in color.h:

    #define GIT_COLOR_NORMAL        ""
    #define GIT_COLOR_RESET         "\033[m"
    #define GIT_COLOR_BOLD          "\033[1m"
    #define GIT_COLOR_RED           "\033[31m"
    #define GIT_COLOR_GREEN         "\033[32m"
    ...

As a workaround, I ended up doing this:

    [color "decorate"]
        head = reset

which should work OK.  But I have a feeling that the definition of
our "normal" may want to do the "reset", not "no-op" like we
currently do.

Comments?

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

[Junio C Hamano]

If you wanted to paint the HEAD and local branch name in the same
color as the body text (perhaps because cyan and green are too faint
on a black-on-white terminal to be readable), you would not want to
have to say

    [color "decorate"]
        head = black
        branch = black

because that you would not be able to reuse same configuration on a
white-on-black terminal.  You would naively expect

    [color "decorate"]
        head = normal
	branch = normal

to work, but unfortunately it does not.  It paints the string "HEAD"
and the branch name in the same color as the opening parenthesis or
comma between the decoration elements when showing output like this:

    $ git show -s --decorate
    commit f3f407747c1cce420ae4b4857c4a6806efe38680 (HEAD, master)
    ...

This is because the code forgets to reset the color after printing
the "prefix" in its own color.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * Answering to myself ...

 log-tree.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/log-tree.c b/log-tree.c
index 7f0890e..53bb526 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -195,6 +195,7 @@ void format_decorations_extended(struct strbuf *sb,
 	while (decoration) {
 		strbuf_addstr(sb, color_commit);
 		strbuf_addstr(sb, prefix);
+		strbuf_addstr(sb, color_reset);
 		strbuf_addstr(sb, decorate_get_color(use_color, decoration->type));
 		if (decoration->type == DECORATION_REF_TAG)
 			strbuf_addstr(sb, "tag: ");

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

[Jeff King]

On Wed, Feb 18, 2015 at 01:44:54PM -0800, Junio C Hamano wrote:

> If you wanted to paint the HEAD and local branch name in the same
> color as the body text (perhaps because cyan and green are too faint
> on a black-on-white terminal to be readable), you would not want to
> have to say
> 
>     [color "decorate"]
>         head = black
>         branch = black
> 
> because that you would not be able to reuse same configuration on a
> white-on-black terminal.  You would naively expect
> 
>     [color "decorate"]
>         head = normal
> 	branch = normal
> 
> to work, but unfortunately it does not.  It paints the string "HEAD"
> and the branch name in the same color as the opening parenthesis or
> comma between the decoration elements when showing output like this:
> 
>     $ git show -s --decorate
>     commit f3f407747c1cce420ae4b4857c4a6806efe38680 (HEAD, master)
>     ...
> 
> This is because the code forgets to reset the color after printing
> the "prefix" in its own color.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---

Yeah, I think this is a good fix. I had a vague feeling that we may have
done this on purpose to let the decoration color "inherit" from the
existing colors for backwards compatibility, but I don't think that
could ever have worked (since color.decorate.* never defaulted to
"normal"). And I couldn't find anything on the list. I think I am
probably thinking of color.diff.func, which faced a similar issue.

Also, for your amusement:

  http://thread.gmane.org/gmane.comp.version-control.git/191102/focus=191118

Believe it or not, this was actually an item on my todo list, which is
perhaps a commentary on how sad and unrealistic my todo list is. But I'm
crossing it off anyway. Task accomplished!

-Peff

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

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Yeah, I think this is a good fix. I had a vague feeling that we may have
> done this on purpose to let the decoration color "inherit" from the
> existing colors for backwards compatibility, but I don't think that
> could ever have worked (since color.decorate.* never defaulted to
> "normal").

Hmph, but that $gmane/191118 talks about giving bold to commit-color
and then expecting for decors to inherit the boldness, a wish I can
understand.  But I do not necessarily agree with it---it relies on
that after "<commit-color>(" and "<commit-color>, " there is no reset,
which is not how everything else works.

So this change at least needs to come with an explanation to people
who are used to and took advantage of this color attribute leakage,
definitely in the log message and preferrably to the documentation
that covers all the color.*.<slot> settings, I think.

Thanks.

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

[Jeff King]

On Thu, Feb 19, 2015 at 10:02:12AM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > Yeah, I think this is a good fix. I had a vague feeling that we may have
> > done this on purpose to let the decoration color "inherit" from the
> > existing colors for backwards compatibility, but I don't think that
> > could ever have worked (since color.decorate.* never defaulted to
> > "normal").
> 
> Hmph, but that $gmane/191118 talks about giving bold to commit-color
> and then expecting for decors to inherit the boldness, a wish I can
> understand.  But I do not necessarily agree with it---it relies on
> that after "<commit-color>(" and "<commit-color>, " there is no reset,
> which is not how everything else works.

I don't see anybody actually _wanting_ the inheritance. It is mentioned
merely as an observation. So yeah, we would break anybody who does:

  [color "diff"]
  commit = blue

  [color "decorate"]
  branch = normal
  remoteBranch = normal
  tag = normal
  stash = normal
  HEAD = normal

and expects the "blue" to persist automatically.

But given that this behaves in the opposite way of every other part of
git's color handling, I think we can call it a bug, and people doing
that are crazy (they should s/normal/blue/ in the latter config).

> So this change at least needs to come with an explanation to people
> who are used to and took advantage of this color attribute leakage,
> definitely in the log message and preferrably to the documentation
> that covers all the color.*.<slot> settings, I think.

I'd agree it is worth a mention in the log (and possibly release notes),
but I don't think it is worth polluting the documentation forever
(though explaining that we never inherit might be worth doing, and that
is perhaps what you meant).

-Peff

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

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I'd agree it is worth a mention in the log (and possibly release notes),
> but I don't think it is worth polluting the documentation forever
> (though explaining that we never inherit might be worth doing, and that
> is perhaps what you meant).

Yes, I do not know how well the attached will render, but something
along the lines of this patch is what I had in mind.

-- >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.

I didn't attempt to de-dup descriptions for boolean and integer
valued variables in this change, but somebody may want to read
the existing descriptions of these variables over and drop them
as necessary.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt | 58 +++++++++++++++++++++++++++++++++++-------------
 1 file changed, 42 insertions(+), 16 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 097fdd4..171287e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -126,6 +126,45 @@ Example
 		path = foo ; expand "foo" relative to the current file
 		path = ~/foo ; expand "foo" in your $HOME directory
 
+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`.
+
+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.
+
+color::
+	The value for a variables that takes a color is a list of
+	colors (at most two) and attributes (at most one), separated
+	by spaces.  The colors accepted are `normal`, `black`,
+	`red`, `green`, `yellow`, `blue`, `magenta`, `cyan` and
+	`white`; the attributes are `bold`, `dim`, `ul`, `blink` and
+	`reverse`.  The first color given is the foreground; the
+	second is the background.  The position of the attribute, if
+	any, doesn't matter.
++
+The attributes are meant to be reset at the beginning of each item
+in the colored output, so setting color.decorate.branch to `black`
+will paint that branch name in a plain `black`, even if the previous
+thing on the same output line (e.g. opening parenthesis before the
+list of branch names in `log --decorate` output) is set to be painted
+with `bold` or some other attribute.
+
+
 Variables
 ~~~~~~~~~
 
@@ -817,14 +856,6 @@ color.branch.<slot>::
 	`remote` (a remote-tracking branch in refs/remotes/),
 	`upstream` (upstream tracking branch), `plain` (other
 	refs).
-+
-The value for these configuration variables is a list of colors (at most
-two) and attributes (at most one), separated by spaces.  The colors
-accepted are `normal`, `black`, `red`, `green`, `yellow`, `blue`,
-`magenta`, `cyan` and `white`; the attributes are `bold`, `dim`, `ul`,
-`blink` and `reverse`.  The first color given is the foreground; the
-second is the background.  The position of the attribute, if any,
-doesn't matter.
 
 color.diff::
 	Whether to use ANSI escape sequences to add color to patches.
@@ -844,8 +875,7 @@ color.diff.<slot>::
 	of `plain` (context text), `meta` (metainformation), `frag`
 	(hunk header), 'func' (function in hunk header), `old` (removed lines),
 	`new` (added lines), `commit` (commit headers), or `whitespace`
-	(highlighting whitespace errors). The values of these variables may be
-	specified as in color.branch.<slot>.
+	(highlighting whitespace errors).
 
 color.decorate.<slot>::
 	Use customized color for 'git log --decorate' output.  `<slot>` is one
@@ -878,8 +908,6 @@ color.grep.<slot>::
 	separators between fields on a line (`:`, `-`, and `=`)
 	and between hunks (`--`)
 --
-+
-The values of these variables may be specified as in color.branch.<slot>.
 
 color.interactive::
 	When set to `always`, always use colors for interactive prompts
@@ -892,8 +920,7 @@ color.interactive.<slot>::
 	Use customized color for 'git add --interactive' and 'git clean
 	--interactive' output. `<slot>` may be `prompt`, `header`, `help`
 	or `error`, for four distinct types of normal output from
-	interactive commands.  The values of these variables may be
-	specified as in color.branch.<slot>.
+	interactive commands.
 
 color.pager::
 	A boolean to enable/disable colored output when the pager is in
@@ -919,8 +946,7 @@ color.status.<slot>::
 	`untracked` (files which are not tracked by Git),
 	`branch` (the current branch), or
 	`nobranch` (the color the 'no branch' warning is shown in, defaulting
-	to red). The values of these variables may be specified as in
-	color.branch.<slot>.
+	to red).
 
 color.ui::
 	This variable determines the default value for variables such

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

[Jeff King]

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

> -- >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...

> +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

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

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Fri, Feb 20, 2015 at 03:06:39PM -0800, Junio C Hamano wrote:
>
>> -- >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?

Yes.

> 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...
>
>> +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:
> ...

Heh, thanks.  That is what I wanted to say when I said "other types
may need de-duping" ;-).

[PATCH v2 0/7] Fix leak of color/attributes in "git log --decorate"

[Junio C Hamano]

In "git log --decorate", you would see the commit header like this:

    commit ... (HEAD, jc/decorate-leaky-separator-color)

where "commit ... (" is painted in color.diff.commit, "HEAD" in
color.decorate.head, ", " in color.diff.commit, the branch name in
color.decorate.branch and then closing ")" in color.diff.commit.

However, setting color.decorate.head to normal does not paint "HEAD"
in the "normal" color you have for your terminal.  It just uses the
same color it used to paint the "(", i.e. color.diff.commit.

Fixing this was a simple one-liner; the code forgot to reset the
terminal attributes before moving on to the next item.

It however turns out that the existing documentation was rather
messy and I ended up doing some preparatory clean-up on the
documentation around how configuration variables are explained
before updating the documentation to clarify that 'normal' ought to
work (in other words, the colors and attributes are not cumulative).

I am reasonably happy with the result, and I can go with or without
[6/7].

The previous round starts at $gmane/264065 [*1*]

Junio C Hamano (7):
  Documentation/config.txt: avoid unnecessary negation
  Documentation/config.txt: explain multi-valued variables once
  Documentation/config.txt: describe the structure first and then meaning
  Documentation/config.txt: have a separate "Values" section
  Documentation/config.txt: describe 'color' value type in the "Values" section
  Documentation/config.txt: simplify boolean description in the syntax section
  log --decorate: do not leak "commit" color into the next item

 Documentation/config.txt         | 111 ++++++++++++++++++++++++---------------
 log-tree.c                       |   1 +
 t/t4207-log-decoration-colors.sh |  16 +++---
 3 files changed, 77 insertions(+), 51 deletions(-)

[Footnote]

*1* http://thread.gmane.org/gmane.comp.version-control.git/264063/focus=264065
    http://mid.gmane.org/xmqqpp9628tl.fsf@gitster.dls.corp.google.com

-- 
2.3.1-316-g7c93423

[PATCH v2 1/7] Documentation/config.txt: avoid unnecessary negation

[Junio C Hamano]

Section names and variable names are both case-insensitive, but one
is described as "not case sensitive".  Use "case-insensitive" for
both.

Instead of saying "... have to be escaped" without telling what that
escaping achieves, state it in a more positive way, i.e. "... can be
included by escaping".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 097fdd4..dbe7035 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -25,7 +25,7 @@ blank lines are ignored.
 
 The file consists of sections and variables.  A section begins with
 the name of the section in square brackets and continues until the next
-section begins.  Section names are not case sensitive.  Only alphanumeric
+section begins.  Section names are case-insensitive.  Only alphanumeric
 characters, `-` and `.` are allowed in section names.  Each variable
 must belong to some section, which means that there must be a section
 header before the first setting of a variable.
@@ -40,8 +40,8 @@ in the section header, like in the example below:
 --------
 
 Subsection names are case sensitive and can contain any characters except
-newline (doublequote `"` and backslash have to be escaped as `\"` and `\\`,
-respectively).  Section headers cannot span multiple
+newline (doublequote `"` and backslash can be included by escaping them
+as `\"` and `\\`, respectively).  Section headers cannot span multiple
 lines.  Variables may belong directly to a section or to a given subsection.
 You can have `[section]` if you have `[section "subsection"]`, but you
 don't need to.
-- 
2.3.1-316-g7c93423

[PATCH v2 2/7] Documentation/config.txt: explain multi-valued variables once

[Junio C Hamano]

The syntax section repeats what the preamble explained already.
That a variable can have multiple values is more about what a
variable is than the syntax of the file.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index dbe7035..405bf25 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -14,7 +14,8 @@ the fully qualified variable name of the variable itself is the last
 dot-separated segment and the section name is everything before the last
 dot. The variable names are case-insensitive, allow only alphanumeric
 characters and `-`, and must start with an alphabetic character.  Some
-variables may appear multiple times.
+variables may appear multiple times; we say then that the variable is
+multivalued.
 
 Syntax
 ~~~~~~
@@ -56,9 +57,7 @@ header) are recognized as setting variables, in the form
 'name = value'.  If there is no equal sign on the line, the entire line
 is taken as 'name' and the variable is recognized as boolean "true".
 The variable names are case-insensitive, allow only alphanumeric characters
-and `-`, and must start with an alphabetic character.  There can be more
-than one value for a given variable; we say then that the variable is
-multivalued.
+and `-`, and must start with an alphabetic character.
 
 Leading and trailing whitespace in a variable value is discarded.
 Internal whitespace within a variable value is retained verbatim.
-- 
2.3.1-316-g7c93423

[PATCH v2 3/7] Documentation/config.txt: describe the structure first and then meaning

[Junio C Hamano]

A line can be continued via a backquote-LF and can be chomped at a
comment character.  But that is not specific to string-typed values.
It is common to all, just like unquoted leading and trailing
whitespaces are stripped and inter-word spacing are retained.

Move the description around and desribe these structural rules
first, then introduce the double-quote facility as a way to override
them, and finally mention various types of values.

Note that these structural rules only apply to the value part of the
configuration file.  E.g.

    [aSection] \
        name \
	= value

does not work, because the rules kick in only after seeing "name =".
Both the original and the updated text are phrased in an awkward way
by singling out the "value" part of the line because of this.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 405bf25..1444614 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -59,32 +59,31 @@ is taken as 'name' and the variable is recognized as boolean "true".
 The variable names are case-insensitive, allow only alphanumeric characters
 and `-`, and must start with an alphabetic character.
 
-Leading and trailing whitespace in a variable value is discarded.
-Internal whitespace within a variable value is retained verbatim.
+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
+verbatim.
 
-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".
-
-String values may be entirely or partially enclosed in double quotes.
-You need to enclose variable values in double quotes if you want to
-preserve leading or trailing whitespace, or if the variable value contains
-comment characters (i.e. it contains '#' or ';').
-Double quote `"` and backslash `\` characters in variable values must
-be escaped: use `\"` for `"` and `\\` for `\`.
+Inside double quotes, double quote `"` and backslash `\` characters
+must be escaped: use `\"` for `"` and `\\` for `\`.
 
 The following escape sequences (beside `\"` and `\\`) are recognized:
 `\n` for newline character (NL), `\t` for horizontal tabulation (HT, TAB)
 and `\b` for backspace (BS).  No other char escape sequence, nor octal
 char sequences are valid.
 
-Variable values ending in a `\` are continued on the next line in the
-customary UNIX fashion.
+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".
 
 Some variables may require a special value format.
 
+
 Includes
 ~~~~~~~~
 
-- 
2.3.1-316-g7c93423

[PATCH v2 4/7] Documentation/config.txt: have a separate "Values" section

[Junio C Hamano]

The various types of values set to the configuration variables
deserve more than a brief footnote mention in the syntax section,
and it will be more so after the later steps of this clean up
effort.

Move the mention of booleans from the syntax section to this new
section, and describe how human-readble integers can be spelled with
scaling there.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt | 39 +++++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1444614..7be608b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -75,14 +75,6 @@ The following escape sequences (beside `\"` and `\\`) are recognized:
 and `\b` for backspace (BS).  No other char escape sequence, nor octal
 char sequences are valid.
 
-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".
-
-Some variables may require a special value format.
-
 
 Includes
 ~~~~~~~~
@@ -124,6 +116,37 @@ Example
 		path = foo ; expand "foo" relative to the current file
 		path = ~/foo ; expand "foo" in your $HOME directory
 
+
+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'; these are all
+       case-insensitive.
+
+       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`.
++
+When converting value to the canonical form using '--bool' type
+specifier; 'git config' will ensure that the output is "true" or
+"false" (spelled in lowercase).
+
+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.
+
+
 Variables
 ~~~~~~~~~
 
-- 
2.3.1-316-g7c93423

[PATCH v2 5/7] Documentation/config.txt: describe 'color' value type in the "Values" section

[Junio C Hamano]

Instead of describing it for color.branch.<slot> and have everybody
else refer to it, explain how colors are spelled in "Values" section
upfront.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 7be608b..c40bf4a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -146,6 +146,16 @@ integer::
        be suffixed with `k`, `M`,... to mean "scale the number by
        1024", "by 1024x1024", etc.
 
+color::
+       The value for a variables that takes a color is a list of
+       colors (at most two) and attributes (at most one), separated
+       by spaces.  The colors accepted are `normal`, `black`,
+       `red`, `green`, `yellow`, `blue`, `magenta`, `cyan` and
+       `white`; the attributes are `bold`, `dim`, `ul`, `blink` and
+       `reverse`.  The first color given is the foreground; the
+       second is the background.  The position of the attribute, if
+       any, doesn't matter.
+
 
 Variables
 ~~~~~~~~~
@@ -838,14 +848,6 @@ color.branch.<slot>::
 	`remote` (a remote-tracking branch in refs/remotes/),
 	`upstream` (upstream tracking branch), `plain` (other
 	refs).
-+
-The value for these configuration variables is a list of colors (at most
-two) and attributes (at most one), separated by spaces.  The colors
-accepted are `normal`, `black`, `red`, `green`, `yellow`, `blue`,
-`magenta`, `cyan` and `white`; the attributes are `bold`, `dim`, `ul`,
-`blink` and `reverse`.  The first color given is the foreground; the
-second is the background.  The position of the attribute, if any,
-doesn't matter.
 
 color.diff::
 	Whether to use ANSI escape sequences to add color to patches.
@@ -865,8 +867,7 @@ color.diff.<slot>::
 	of `plain` (context text), `meta` (metainformation), `frag`
 	(hunk header), 'func' (function in hunk header), `old` (removed lines),
 	`new` (added lines), `commit` (commit headers), or `whitespace`
-	(highlighting whitespace errors). The values of these variables may be
-	specified as in color.branch.<slot>.
+	(highlighting whitespace errors).
 
 color.decorate.<slot>::
 	Use customized color for 'git log --decorate' output.  `<slot>` is one
@@ -899,8 +900,6 @@ color.grep.<slot>::
 	separators between fields on a line (`:`, `-`, and `=`)
 	and between hunks (`--`)
 --
-+
-The values of these variables may be specified as in color.branch.<slot>.
 
 color.interactive::
 	When set to `always`, always use colors for interactive prompts
@@ -913,8 +912,7 @@ color.interactive.<slot>::
 	Use customized color for 'git add --interactive' and 'git clean
 	--interactive' output. `<slot>` may be `prompt`, `header`, `help`
 	or `error`, for four distinct types of normal output from
-	interactive commands.  The values of these variables may be
-	specified as in color.branch.<slot>.
+	interactive commands.
 
 color.pager::
 	A boolean to enable/disable colored output when the pager is in
@@ -940,8 +938,7 @@ color.status.<slot>::
 	`untracked` (files which are not tracked by Git),
 	`branch` (the current branch), or
 	`nobranch` (the color the 'no branch' warning is shown in, defaulting
-	to red). The values of these variables may be specified as in
-	color.branch.<slot>.
+	to red).
 
 color.ui::
 	This variable determines the default value for variables such
-- 
2.3.1-316-g7c93423

[PATCH v2 6/7] Documentation/config.txt: simplify boolean description in the syntax section

[Junio C Hamano]

The 'true' short-hand doesn't deserve a separate sentence; even our own

    git config --bool foo.bar yes

would not produce it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index c40bf4a..f1cf571 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -54,8 +54,8 @@ restrictions as section names.
 
 All the other lines (and the remainder of the line after the section
 header) are recognized as setting variables, in the form
-'name = value'.  If there is no equal sign on the line, the entire line
-is taken as 'name' and the variable is recognized as boolean "true".
+'name = value' (or just 'name', which is a short-hand to say that
+the variable is the boolean "true").
 The variable names are case-insensitive, allow only alphanumeric characters
 and `-`, and must start with an alphabetic character.
 
-- 
2.3.1-316-g7c93423

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

[Junio C Hamano]

In "git log --decorate", you would see the commit header like this:

    commit ... (HEAD, jc/decorate-leaky-separator-color)

where "commit ... (" is painted in color.diff.commit, "HEAD" in
color.decorate.head, ", " in color.diff.commit, the branch name in
color.decorate.branch and then closing ")" in color.diff.commit.

If you wanted to paint the HEAD and local branch name in the same
color as the body text (perhaps because cyan and green are too faint
on a black-on-white terminal to be readable), you would not want to
have to say

    [color "decorate"]
        head = black
        branch = black

because that you would not be able to reuse same configuration on a
white-on-black terminal.  You would naively expect

    [color "decorate"]
        head = normal
	branch = normal

to work, but unfortunately it does not.  It paints the string "HEAD"
and the branch name in the same color as the opening parenthesis or
comma between the decoration elements.  This is because the code
forgets to reset the color after printing the "prefix" in its own
color.

It theoretically is possible that some people were expecting and
relying on that the attribute set as the "diff.commit" color, which
is used to draw these opening parenthesis and inter-item comma, is
inherited by the drawing of branch names, but it is not how the
coloring works everywhere else.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt         |  7 +++++++
 log-tree.c                       |  1 +
 t/t4207-log-decoration-colors.sh | 16 ++++++++--------
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f1cf571..6d65033 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -155,6 +155,13 @@ color::
        `reverse`.  The first color given is the foreground; the
        second is the background.  The position of the attribute, if
        any, doesn't matter.
++
+The attributes are meant to be reset at the beginning of each item
+in the colored output, so setting color.decorate.branch to `black`
+will paint that branch name in a plain `black`, even if the previous
+thing on the same output line (e.g. opening parenthesis before the
+list of branch names in `log --decorate` output) is set to be
+painted with `bold` or some other attribute.
 
 
 Variables
diff --git a/log-tree.c b/log-tree.c
index 1982631..11676d5 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -200,6 +200,7 @@ void format_decorations(struct strbuf *sb,
 	while (decoration) {
 		strbuf_addstr(sb, color_commit);
 		strbuf_addstr(sb, prefix);
+		strbuf_addstr(sb, color_reset);
 		strbuf_addstr(sb, decorate_get_color(use_color, decoration->type));
 		if (decoration->type == DECORATION_REF_TAG)
 			strbuf_addstr(sb, "tag: ");
diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
index 925f577..6b8ad4f 100755
--- a/t/t4207-log-decoration-colors.sh
+++ b/t/t4207-log-decoration-colors.sh
@@ -44,15 +44,15 @@ test_expect_success setup '
 '
 
 cat >expected <<EOF
-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_HEAD}HEAD${c_reset}${c_commit},\
- ${c_tag}tag: v1.0${c_reset}${c_commit},\
- ${c_tag}tag: B${c_reset}${c_commit},\
- ${c_branch}master${c_reset}${c_commit})${c_reset} B
-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_tag}tag: A1${c_reset}${c_commit},\
- ${c_remoteBranch}other/master${c_reset}${c_commit})${c_reset} A1
-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_stash}refs/stash${c_reset}${c_commit})${c_reset}\
+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD${c_reset}${c_commit},\
+ ${c_reset}${c_tag}tag: v1.0${c_reset}${c_commit},\
+ ${c_reset}${c_tag}tag: B${c_reset}${c_commit},\
+ ${c_reset}${c_branch}master${c_reset}${c_commit})${c_reset} B
+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A1${c_reset}${c_commit},\
+ ${c_reset}${c_remoteBranch}other/master${c_reset}${c_commit})${c_reset} A1
+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_stash}refs/stash${c_reset}${c_commit})${c_reset}\
  On master: Changes to A.t
-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_tag}tag: A${c_reset}${c_commit})${c_reset} A
+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A${c_reset}${c_commit})${c_reset} A
 EOF
 
 # We want log to show all, but the second parent to refs/stash is irrelevant
-- 
2.3.1-316-g7c93423