git diff-index ignores color config

git diff-index ignores color config

[Elias Persson]

Having color.ui (and/or color.diff) set to auto, doing

   git diff-index --check <some refspec>

on a set with e.g. trailing whitespace does not produce colored
output.

If providing the option on the command line, e.g.

   git diff-index --check --color=auto <some refspec>

output _is_ colored, so obviously not an issue with tty
detection. Doesn't work when configured to 'true' either.


Documentation indicates that the config option is considered
(see under --no-color), so to me it seems like there's a bug
here, somewhere.


Most likely way of coming across this issue would be turning on
the shipped pre-commit hook sample, at least that's how I found
it. It ends (and does most of its work) with

    exec git diff-index --check --cached $against --

where $against is normally HEAD.


git --version: 1.7.4.4
uname -srio: Linux 2.6.35.12-88.fc14.i686 i386 GNU/Linux


// Delreich

Re: git diff-index ignores color config

[Junio C Hamano]

Elias Persson <delreich@takeit.se> writes:

> Having color.ui (and/or color.diff) set to auto, doing
>
>    git diff-index --check <some refspec>
>
> on a set with e.g. trailing whitespace does not produce colored
> output.

Isn't that very much on purpose?  diff-{index,tree,files} plumbings should
not be affected by UI related configuration variables.

Re: git diff-index ignores color config

[Elias Persson]

On Tue, 2011-04-26 at 10:42 -0700, Junio C Hamano wrote:
> Isn't that very much on purpose?  diff-{index,tree,files} plumbings should
> not be affected by UI related configuration variables.

Documentation indicates that color is configurable for it though.
I can't say if the issue is with the behavior or the documentation.

I can say I find the pre-commit verifying hook to be a lot more
useful when you can actually see the whitespace issues.

[PATCH] config.txt,diff-options.txt: porcelain vs. plumbing for color.diff

[Michael J Gruber]

Reading the diff-family and config man pages one may think that the
color.diff and color.ui settings apply to all diff commands. Make it
clearer that they do not apply to the plumbing variants
diff-{files,index,tree}.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 Documentation/config.txt       |    4 ++--
 Documentation/diff-options.txt |   15 +++++++++++----
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 750c86d..1e22832 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -708,7 +708,7 @@ second is the background.  The position of the attribute, if any,
 doesn't matter.
 
 color.diff::
-	When set to `always`, always use colors in patch.
+	When set to `always`, always use colors in patch for porcelain commands.
 	When false (or `never`), never.  When set to `true` or `auto`, use
 	colors only when the output is to the terminal. Defaults to false.
 
@@ -796,7 +796,7 @@ color.status.<slot>::
 	color.branch.<slot>.
 
 color.ui::
-	When set to `always`, always use colors in all git commands which
+	When set to `always`, always use colors in all porcelain commands which
 	are capable of colored output. When false (or `never`), never. When
 	set to `true` or `auto`, use colors only when the output is to the
 	terminal. When more specific variables of color.* are set, they always
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index c93124b..c26e494 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -120,12 +120,19 @@ any of those replacements occurred.
 
 --color[=<when>]::
 	Show colored diff.
-	The value must be always (the default), never, or auto.
+	The value must be `always` (the default for `<when>`), `never`, or `auto`.
+	The default value is `never`.
+ifdef::git-diff[]
+	It can be changed by the `color.ui` and `color.diff`
+	configuration settings.
+endif::git-diff[]
 
 --no-color::
-	Turn off colored diff, even when the configuration file
-	gives the default to color output.
-	Same as `--color=never`.
+	Turn off colored diff.
+ifdef::git-diff[]
+	This can be used to override configuration settings.
+endif::git-diff[]
+	It is the same as `--color=never`.
 
 --word-diff[=<mode>]::
 	Show a word diff, using the <mode> to delimit changed words.
-- 
1.7.5.270.gafca7

Re: [PATCH] config.txt,diff-options.txt: porcelain vs. plumbing for color.diff

[Jonathan Nieder]

Michael J Gruber wrote:

> Reading the diff-family and config man pages one may think that the
> color.diff and color.ui settings apply to all diff commands. Make it
> clearer that they do not apply to the plumbing variants
> diff-{files,index,tree}.

Sounds like a good idea.  Quick reactions:

> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -708,7 +708,7 @@ second is the background.  The position of the attribute, if any,
>  doesn't matter.
>  
>  color.diff::
> -	When set to `always`, always use colors in patch.
> +	When set to `always`, always use colors in patch for porcelain commands.
>  	When false (or `never`), never.  When set to `true` or `auto`, use
>  	colors only when the output is to the terminal. Defaults to false.

I am not sure I like promising that all porcelain commands will use
color; for example, "git commit -v" does not (though "commit -v
--dry-run" does).  Maybe:

	Whether to use ANSI escape sequences to add color to patches.
	If this is set to `always`, linkgit:git-diff[1],
	linkgit:git-log[1], and linkgit:git-show[1] will use color
	for all patches.  If it is set to `true` or `auto`, those
	commands will only use color when output is to the terminal.
	Defaults to false.
+
This does not affect linkgit:git-format-patch[1] or the
'git-diff-{asterisk}' plumbing commands.  Can be overridden on the
command line with the `--color[=<when>]` option.

> @@ -796,7 +796,7 @@ color.status.<slot>::
>  	color.branch.<slot>.
>  
>  color.ui::
> -	When set to `always`, always use colors in all git commands which
> +	When set to `always`, always use colors in all porcelain commands which
>  	are capable of colored output. When false (or `never`), never. When

Nitpick: the grammatical subject of the dependent clause ("when set to
always") and the rest of the sentence ("always use colors") differ, so
the sentence sounds awkward.  Not your fault, but while we're here, it
might be nice to change that.

I think the intent is something like

	This variable determines the default for color.branch,
	color.diff, color.grep, color.interactive, color.showbranch,
	and color.status, and its scope will expand as other commands
	learn configuration to set a default for the --color option.
	Set it to "always" if you want all output not intended for
	machine consumption to use color, to "true" or "auto" if you
	want such output to use color when written to the terminal,
	or to "false" or "never" if you prefer git commands not to
	use color unless enabled explicitly with some other
	configuration or the --color option.

In other words, it's closer to "all appropriate commands" than "all
porcelain".  format-patch is porcelain.  Not sure what a concise way
to say that is, though.

Thanks, and hope that helps.
Jonathan

Re: [PATCH] config.txt,diff-options.txt: porcelain vs. plumbing for color.diff

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Michael J Gruber wrote:
>
>> Reading the diff-family and config man pages one may think that the
>> color.diff and color.ui settings apply to all diff commands. Make it
>> clearer that they do not apply to the plumbing variants
>> diff-{files,index,tree}.
>
> Sounds like a good idea.  Quick reactions:
> ...
> Thanks, and hope that helps.

Your text reads better at least to me.

Re: [PATCH] config.txt,diff-options.txt: porcelain vs. plumbing for color.diff

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Michael J Gruber wrote:
>>
>>> Reading the diff-family and config man pages one may think that the
>>> color.diff and color.ui settings apply to all diff commands. Make it
>>> clearer that they do not apply to the plumbing variants
>>> diff-{files,index,tree}.
>>
>> Sounds like a good idea.  Quick reactions:
>> ...
>> Thanks, and hope that helps.
>
> Your text reads better at least to me.

I'll queue the following.

-- >8 --
From: Michael J Gruber <git@drmicha.warpmail.net>
Subject: [PATCH] config.txt,diff-options.txt: porcelain vs. plumbing for color.diff

Reading the diff-family and config man pages one may think that the
color.diff and color.ui settings apply to all diff commands. Make it
clearer that they do not apply to the plumbing variants
diff-{files,index,tree}.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt       |   27 +++++++++++++++++++--------
 Documentation/diff-options.txt |   15 +++++++++++----
 2 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 750c86d..3967b1a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -708,9 +708,16 @@ second is the background.  The position of the attribute, if any,
 doesn't matter.
 
 color.diff::
-	When set to `always`, always use colors in patch.
-	When false (or `never`), never.  When set to `true` or `auto`, use
-	colors only when the output is to the terminal. Defaults to false.
+	Whether to use ANSI escape sequences to add color to patches.
+	If this is set to `always`, linkgit:git-diff[1],
+	linkgit:git-log[1], and linkgit:git-show[1] will use color
+	for all patches.  If it is set to `true` or `auto`, those
+	commands will only use color when output is to the terminal.
+	Defaults to false.
++
+This does not affect linkgit:git-format-patch[1] nor the
+'git-diff-{asterisk}' plumbing commands.  Can be overridden on the
+command line with the `--color[=<when>]` option.
 
 color.diff.<slot>::
 	Use customized color for diff colorization.  `<slot>` specifies
@@ -796,11 +803,15 @@ color.status.<slot>::
 	color.branch.<slot>.
 
 color.ui::
-	When set to `always`, always use colors in all git commands which
-	are capable of colored output. When false (or `never`), never. When
-	set to `true` or `auto`, use colors only when the output is to the
-	terminal. When more specific variables of color.* are set, they always
-	take precedence over this setting. Defaults to false.
+	This variable determines the default value for variables such
+	as `color.diff` and `color.grep` that control the use of color
+	per command family. Its scope will expand as more commands learn
+	configuration to set a default for the `--color` option.  Set it
+	to `always` if you want all output not intended for machine
+	consumption to use color, to `true` or `auto` if you want such
+	output to use color when written to the terminal, or to `false` or
+	`never` if you prefer git commands not to use color unless enabled
+	explicitly with some other configuration or the `--color` option.
 
 commit.status::
 	A boolean to enable/disable inclusion of status information in the
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index c93124b..c26e494 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -120,12 +120,19 @@ any of those replacements occurred.
 
 --color[=<when>]::
 	Show colored diff.
-	The value must be always (the default), never, or auto.
+	The value must be `always` (the default for `<when>`), `never`, or `auto`.
+	The default value is `never`.
+ifdef::git-diff[]
+	It can be changed by the `color.ui` and `color.diff`
+	configuration settings.
+endif::git-diff[]
 
 --no-color::
-	Turn off colored diff, even when the configuration file
-	gives the default to color output.
-	Same as `--color=never`.
+	Turn off colored diff.
+ifdef::git-diff[]
+	This can be used to override configuration settings.
+endif::git-diff[]
+	It is the same as `--color=never`.
 
 --word-diff[=<mode>]::
 	Show a word diff, using the <mode> to delimit changed words.
-- 
1.7.5.208.g75460

Re: [PATCH] config.txt,diff-options.txt: porcelain vs. plumbing for color.diff

[Jonathan Nieder]

Junio C Hamano wrote:

> I'll queue the following.

Thanks; looks good.

Re: [PATCH] config.txt,diff-options.txt: porcelain vs. plumbing for color.diff

[Michael J Gruber]

Jonathan Nieder venit, vidit, dixit 28.04.2011 00:12:
> Junio C Hamano wrote:
> 
>> I'll queue the following.
> 
> Thanks; looks good.

Yep, thanks both of you!

Michael