Re: What happened to "git status --color=(always|auto|never)"?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Oct 10, 2017 at 12:42:43PM +0800, Nazri Ramliy wrote:

> >         commit 6be4595edb8e5b616c6e8b9fbc78b0f831fa2a87
> >         Author: Jeff King <peff@xxxxxxxx>
> >         Date:   Tue Oct 3 09:46:06 2017 -0400
> >
> >             color: make "always" the same as "auto" in config
> >
> > Would you like to take a stab at adding it?  builtin/commit.c and
> > Documentation/git-{commit,status}.txt would be my best guesses at
> > where to start.
> 
> Perhaps, seeing that that commit intentionally "broke" the color
> output of my tool[1], because it parses the output of `git -c
> color.status=always status`, which now no longer work the way it used
> to. I know, I know... shame on me for parsing the output of a
> porcelain command :)
> 
> But this also means that I will have to modify [1] to cope with this,
> given that it may be used with an older version of git (parse
> git-version and shell out to different git command - either `git -c
> color.ui=always status`, or the not-yet-exist `git status
> --color=always`), or make it use the plumbing output of `git status`,
> but that would just add additional work that  I really don't look
> forward to doing at this moment.

:( I was worried that this might hit some third-party scripts.

One workaround you can do that should work with any version of Git is:

  GIT_PAGER_IN_USE=1 git status | your-parser

That tells Git that even though stdout isn't a tty, you're expecting to
present the data to the user and it should be colored appropriately. It
has the additional upside that it doesn't override the user's color
config.

It does have the potential downside that other non-color changes could
kick in (e.g., somebody recently proposed that auto-columns kick in with
GIT_PAGER_IN_USE).

All that said, should we revisit the decision from 6be4595edb? The two
code changes we could make are:

  1. Adding a "--color" option to "git status". Commit 0c88bf5050
     (provide --color option for all ref-filter users, 2017-10-03) from
     that same series shows some prior art.

     This is a clean solution, but it does mean that scripts have to
     adapt (and would potentially need to care about which Git version
     they're relying on).

  2. Re-allow "color.always" config from the command-line. It's actually
     on-disk config that we want to downgrade, but I wanted to avoid
     making complicated rules about how the config would behave in
     different scopes. The patch for this would look something like the
     one below.

  3. Revert the original series, and revisit the original "respect
     color.ui via porcelain" commit which broke add--interactive in
     v2.14.2 (136c8c8b8fa).

I dunno. I think for your use case, PAGER_IN_USE may actually be the
"right" solution, because it most closely expresses what you're doing.
We probably ought to have (1) as a general rule for commands which
handle color.

But (2) and (3) are the only ones that will work seamlessly with
existing scripts. I'm not excited about either of them, though.

-Peff

diff --git a/color.c b/color.c
index 9c0dc82370..3870d3e395 100644
--- a/color.c
+++ b/color.c
@@ -307,8 +307,21 @@ int git_config_colorbool(const char *var, const char *value)
 	if (value) {
 		if (!strcasecmp(value, "never"))
 			return 0;
-		if (!strcasecmp(value, "always"))
-			return var ? GIT_COLOR_AUTO : 1;
+		if (!strcasecmp(value, "always")) {
+			/*
+			 * Command-line options always respect "always".
+			 * Likewise for "-c" config on the command-line.
+			 */
+			if (!var ||
+			    current_config_scope() == CONFIG_SCOPE_CMDLINE)
+				return 1;
+
+			/*
+			 * Otherwise, we're looking at on-disk config;
+			 * downgrade to auto.
+			 */
+			return GIT_COLOR_AUTO;
+		}
 		if (!strcasecmp(value, "auto"))
 			return GIT_COLOR_AUTO;
 	}



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux