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;
}
