[PATCH] Allow subcommand.color and color.subcommand color configuration

[PATCH] Allow subcommand.color and color.subcommand color configuration

[Andy Parkins]

While adding colour to the branch command it was pointed out that a
config option like "branch.color" conflicts with the pre-existing
"branch.something" namespace used for specifying default merge urls and
branches.  The suggested solution was to flip the order of the
components to "color.branch", which I did for colourising branch.

This patch does the same thing for
  - git-log (color.diff)
  - git-status (color.status)
  - git-diff (color.diff)
  - pager (color.pager)

I haven't removed the old config options; but they should probably be
deprecated and eventually removed to prevent future namespace
collisions.  I've done this deprecation by changing the documentation
for the config file to match the new names; and adding the "color.XXX"
options to contrib/completion/git-completion.bash.

Unfortunately git-svn reads "diff.color" and "pager.color"; which I
don't like to change unilaterally.

Signed-off-by: Andy Parkins <andyparkins@gmail.com>
---
 Documentation/config.txt               |   12 ++++++------
 builtin-log.c                          |    2 +-
 config.c                               |    2 +-
 contrib/completion/git-completion.bash |    3 +++
 diff.c                                 |    4 ++--
 wt-status.c                            |    4 ++--
 6 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 9090762..9bdd824 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -130,16 +130,16 @@ branch.<name>.merge::
 	When in branch <name>, it tells `git fetch` the default remote branch
 	to be merged.
 
-pager.color::
+color.pager::
 	A boolean to enable/disable colored output when the pager is in
 	use (default is true).
 
-diff.color::
+color.diff::
 	When true (or `always`), always use colors in patch.
 	When false (or `never`), never.  When set to `auto`, use
 	colors only when the output is to the terminal.
 
-diff.color.<slot>::
+color.diff.<slot>::
 	Use customized color for diff colorization.  `<slot>`
 	specifies which part of the patch to use the specified
 	color, and is one of `plain` (context text), `meta`
@@ -264,19 +264,19 @@ showbranch.default::
 	The default set of branches for gitlink:git-show-branch[1].
 	See gitlink:git-show-branch[1].
 
-status.color::
+color.status::
 	A boolean to enable/disable color in the output of
 	gitlink:git-status[1]. May be set to `true` (or `always`),
 	`false` (or `never`) or `auto`, in which case colors are used
 	only when the output is to a terminal. Defaults to false.
 
-status.color.<slot>::
+color.status.<slot>::
 	Use customized color for status colorization. `<slot>` is
 	one of `header` (the header text of the status message),
 	`updated` (files which are updated but not committed),
 	`changed` (files which are changed but not updated in the index),
 	or `untracked` (files which are not tracked by git). The values of
-	these variables may be specified as in diff.color.<slot>.
+	these variables may be specified as in color.diff.<slot>.
 
 tar.umask::
 	By default, gitlink:git-tar-tree[1] sets file and directories modes
diff --git a/builtin-log.c b/builtin-log.c
index 7acf5d3..6821a08 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -118,7 +118,7 @@ static int git_format_config(const char *var, const char *value)
 		strcat(extra_headers, value);
 		return 0;
 	}
-	if (!strcmp(var, "diff.color")) {
+	if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) {
 		return 0;
 	}
 	return git_log_config(var, value);
diff --git a/config.c b/config.c
index 3cae390..06e7fdb 100644
--- a/config.c
+++ b/config.c
@@ -314,7 +314,7 @@ int git_default_config(const char *var, const char *value)
 		return 0;
 	}
 
-	if (!strcmp(var, "pager.color")) {
+	if (!strcmp(var, "pager.color") || !strcmp(car, "color.pager")) {
 		pager_use_color = git_config_bool(var,value);
 		return 0;
 	}
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 447ec20..9c4d23a 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -712,10 +712,13 @@ _git_repo_config ()
 		core.legacyHeaders
 		i18n.commitEncoding
 		diff.color
+		color.diff
 		diff.renameLimit
 		diff.renames
 		pager.color
+		color.pager
 		status.color
+		color.status
 		log.showroot
 		show.difftree
 		showbranch.default
diff --git a/diff.c b/diff.c
index 3315378..726b01e 100644
--- a/diff.c
+++ b/diff.c
@@ -60,7 +60,7 @@ int git_diff_ui_config(const char *var, const char *value)
 		diff_rename_limit_default = git_config_int(var, value);
 		return 0;
 	}
-	if (!strcmp(var, "diff.color")) {
+	if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) {
 		diff_use_color_default = git_config_colorbool(var, value);
 		return 0;
 	}
@@ -74,7 +74,7 @@ int git_diff_ui_config(const char *var, const char *value)
 			diff_detect_rename_default = DIFF_DETECT_RENAME;
 		return 0;
 	}
-	if (!strncmp(var, "diff.color.", 11)) {
+	if (!strncmp(var, "diff.color.", 11) || !strncmp(var, "color.diff.", 11)) {
 		int slot = parse_diff_color_slot(var, 11);
 		color_parse(value, var, diff_colors[slot]);
 		return 0;
diff --git a/wt-status.c b/wt-status.c
index de1be5b..df582a0 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -297,11 +297,11 @@ void wt_status_print(struct wt_status *s)
 
 int git_status_config(const char *k, const char *v)
 {
-	if (!strcmp(k, "status.color")) {
+	if (!strcmp(k, "status.color") || !strcmp(k, "color.status")) {
 		wt_status_use_color = git_config_colorbool(k, v);
 		return 0;
 	}
-	if (!strncmp(k, "status.color.", 13)) {
+	if (!strncmp(k, "status.color.", 13) || !strncmp(k, "color.status", 13)) {
 		int slot = parse_status_slot(k, 13);
 		color_parse(v, k, wt_status_colors[slot]);
 	}
-- 
1.4.4.1.g3ece-dirty

Re: [PATCH] Allow subcommand.color and color.subcommand color configuration

[Junio C Hamano]

Andy Parkins <andyparkins@gmail.com> writes:

> While adding colour to the branch command it was pointed out that a
> config option like "branch.color" conflicts with the pre-existing
> "branch.something" namespace used for specifying default merge urls and
> branches.  The suggested solution was to flip the order of the
> components to "color.branch", which I did for colourising branch.
> ...
> Unfortunately git-svn reads "diff.color" and "pager.color"; which I
> don't like to change unilaterally.

I think doing the same makes sense.  Something like this?

-- >8 --
git-svn: allow both diff.color and color.diff

The list concensus is to group color related configuration under
"color.*" so let's be consistent.

Inspired by Andy Parkins's patch to do the same for diff/log
family.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
diff --git a/git-svn.perl b/git-svn.perl
index ec92b44..2893e3b 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -925,19 +925,38 @@ sub cmt_showable {
 
 sub log_use_color {
 	return 1 if $_color;
-	my $dc;
-	chomp($dc = `git-repo-config --get diff.color`);
+	my ($dc, $dcvar);
+	$dcvar = 'color.diff'
+	$dc = `git-repo-config --get $dcvar`;
+	if ($dc eq '') {
+		# nothing at all; fallback to "diff.color"
+		$dcvar = 'diff.color';
+		$dc = `git-repo-config --get $dcvar`;
+	}
+	chomp($dc);
 	if ($dc eq 'auto') {
-		if (-t *STDOUT || (defined $_pager &&
-		    `git-repo-config --bool --get pager.color` !~ /^false/)) {
+		my $pc;
+		$pc = `git-repo-config --get color.pager`;
+		if ($pc eq '') {
+			# does not have it -- fallback to pager.color
+			$pc = `git-repo-config --bool --get pager.color`;
+		}
+		else {
+			$pc = `git-repo-config --bool --get color.pager`;
+			if ($?) {
+				$pc = 'false';
+			}
+		}
+		chomp($pc);
+		if (-t *STDOUT || (defined $_pager && $pc eq 'true') {
 			return ($ENV{TERM} && $ENV{TERM} ne 'dumb');
 		}
 		return 0;
 	}
 	return 0 if $dc eq 'never';
 	return 1 if $dc eq 'always';
-	chomp($dc = `git-repo-config --bool --get diff.color`);
-	$dc eq 'true';
+	chomp($dc = `git-repo-config --bool --get $dcvar`);
+	return ($dc eq 'true');
 }
 
 sub git_svn_log_cmd {

Re: [PATCH] Allow subcommand.color and color.subcommand color configuration

[Andy Parkins]

On Wednesday 2006, December 13 22:16, Junio C Hamano wrote:

> I think doing the same makes sense.  Something like this?

Perfect - yes that is the solution.


Andy

-- 
Dr Andrew Parkins, M Eng (Hons), AMIEE

Re: [PATCH] Allow subcommand.color and color.subcommand color configuration

[Eric Wong]

Junio C Hamano <junkio@cox.net> wrote:
> Andy Parkins <andyparkins@gmail.com> writes:
> 
> > While adding colour to the branch command it was pointed out that a
> > config option like "branch.color" conflicts with the pre-existing
> > "branch.something" namespace used for specifying default merge urls and
> > branches.  The suggested solution was to flip the order of the
> > components to "color.branch", which I did for colourising branch.
> > ...
> > Unfortunately git-svn reads "diff.color" and "pager.color"; which I
> > don't like to change unilaterally.
> 
> I think doing the same makes sense.  Something like this?

It works great for me after I fixed some syntax errors (patch below).

> -- >8 --
> git-svn: allow both diff.color and color.diff
> 
> The list concensus is to group color related configuration under
> "color.*" so let's be consistent.
> 
> Inspired by Andy Parkins's patch to do the same for diff/log
> family.
> 
> Signed-off-by: Junio C Hamano <junkio@cox.net>

Signed-off-by: Eric Wong <normalperson@yhbt.net> (with the following
fixes of course :)

diff --git a/git-svn.perl b/git-svn.perl
index 2893e3b..73ab8d8 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -926,7 +926,7 @@ sub cmt_showable {
 sub log_use_color {
 	return 1 if $_color;
 	my ($dc, $dcvar);
-	$dcvar = 'color.diff'
+	$dcvar = 'color.diff';
 	$dc = `git-repo-config --get $dcvar`;
 	if ($dc eq '') {
 		# nothing at all; fallback to "diff.color"
@@ -948,7 +948,7 @@ sub log_use_color {
 			}
 		}
 		chomp($pc);
-		if (-t *STDOUT || (defined $_pager && $pc eq 'true') {
+		if (-t *STDOUT || (defined $_pager && $pc eq 'true')) {
 			return ($ENV{TERM} && $ENV{TERM} ne 'dumb');
 		}
 		return 0;

--