git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Nazri Ramliy <ayiehere@gmail.com>
Cc: Jonathan Nieder <jrnieder@gmail.com>,
	Git Mailing List <git@vger.kernel.org>
Subject: Re: What happened to "git status --color=(always|auto|never)"?
Date: Tue, 10 Oct 2017 06:25:10 -0400	[thread overview]
Message-ID: <20171010102509.e7ucbyon6ka6722l@sigill.intra.peff.net> (raw)
In-Reply-To: <CAEY4ZpMKE6yf2baaJt+x6c_esorFnyWvLZ=_KS1iRs6XbL42hw@mail.gmail.com>

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

> >         commit 6be4595edb8e5b616c6e8b9fbc78b0f831fa2a87
> >         Author: Jeff King <peff@peff.net>
> >         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;
 	}

  reply	other threads:[~2017-10-10 10:25 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-09 23:58 What happened to "git status --color=(always|auto|never)"? Nazri Ramliy
2017-10-10  0:16 ` Jonathan Nieder
2017-10-10  0:43   ` Nazri Ramliy
2017-10-10  0:59     ` Jonathan Nieder
2017-10-10  4:42       ` Nazri Ramliy
2017-10-10 10:25         ` Jeff King [this message]
2017-10-10 12:51           ` Junio C Hamano
2017-10-10 13:06             ` Jeff King
2017-10-10 19:03               ` Jonathan Nieder
2017-10-10 19:37                 ` Jeff King
2017-10-11  2:05                   ` Junio C Hamano
2017-10-12  2:10                     ` [PATCH 0/2] Piling more kludge on top of color.ui Junio C Hamano
2017-10-12  2:10                       ` [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration Junio C Hamano
2017-10-12  4:47                         ` Jonathan Nieder
2017-10-12  5:05                           ` Junio C Hamano
2017-10-12  5:40                             ` Jonathan Nieder
2017-10-12  6:15                               ` Junio C Hamano
2017-10-12  6:58                                 ` Junio C Hamano
2017-10-12 13:06                                   ` Jeff King
2017-10-12 15:12                                     ` Jeff King
2017-10-12 12:31                         ` Jeff King
2017-10-13  0:09                           ` Junio C Hamano
2017-10-13  1:47                             ` Jeff King
2017-10-13  3:37                               ` Junio C Hamano
2017-10-13 13:06                                 ` Jeff King
2017-10-13 17:20                                   ` [PATCH 0/4] peeling back color.ui=always hacks Jeff King
2017-10-13 17:23                                     ` [PATCH 1/4] Revert "color: make "always" the same as "auto" in config" Jeff King
2017-10-13 17:23                                     ` [PATCH 2/4] Revert "t6006: drop "always" color config tests" Jeff King
2017-10-13 17:24                                     ` [PATCH 3/4] Revert "color: check color.ui in git_default_config()" Jeff King
2017-10-13 17:26                                     ` [PATCH 4/4] tag: respect color.ui config Jeff King
2017-10-14  3:01                                   ` [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration Junio C Hamano
2017-10-16 21:53                                     ` Jeff King
2017-10-17  1:06                                       ` Junio C Hamano
2017-10-17  6:26                                         ` Junio C Hamano
2017-10-18  5:28                                           ` Jeff King
2017-10-18  5:57                                             ` Junio C Hamano
2017-10-17  6:51                                         ` Jonathan Nieder
2017-10-18  5:34                                           ` Jeff King
2017-10-12  2:10                       ` [PATCH 2/2] color: discourage use of ui.color=always Junio C Hamano
2017-10-12  4:48                         ` Jonathan Nieder
2017-10-12 15:08                         ` Jeff King
2017-10-13  0:02                           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171010102509.e7ucbyon6ka6722l@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=ayiehere@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).