git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>,
	Nazri Ramliy <ayiehere@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 12:03:14 -0700	[thread overview]
Message-ID: <20171010190314.GW19555@aiede.mtv.corp.google.com> (raw)
In-Reply-To: <20171010130602.ivhsbu2ymnzt7gko@sigill.intra.peff.net>

Hi,

Jeff King wrote:
> On Tue, Oct 10, 2017 at 09:51:38PM +0900, Junio C Hamano wrote:

>> I think the right fix to the original problem (you cannot remove
>> auto-color from the plumbing) is to stop paying attention to color
>> configuration from the default config.  I wonder if something like
>> this would work?
>>
>>  - Initialize color.c::git_use_color_default to GIT_COLOR_UNKNOWN;
>>
>>  - When git_color_config() is called, and if git_use_color_default
>>    is still GIT_COLOR_UNKNOWN, set it to GIT_COLOR_AUTO (regardless
>>    of the variable git_color_config() is called for).
>>
>>  - In color.c::want_color(), when git_use_color_default is used,
>>    notice if it is GIT_COLOR_UNKNOWN and behave as if it is
>>    GIT_COLOR_NEVER.
>>
>> Then we make sure that git_color_config() is never called by any
>> plumbing command.  The fact it is (ever) called can be taken as a
>> clue that we are running a Porcelain (hence we transition from
>> UNKNOWN to AUTO), so we'd get the desirable "no default color for
>> plumbing, auto color for Porcelain", I would think.
>
> Yes, I think that's the simplest way to implement the "plumbing should
> never do color without a command-line option" scheme.
>
> I do wonder if people would end up seeing some corner cases as
> regressions, though. Right now "diff-tree" _does_ color the output by
> default, and it would stop doing so under your scheme. That's the right
> thing to do by the plumbing/porcelain distinction, but users with
> scripts that use diff-tree (or other plumbing) to generate user-visible
> output may unexpectedly lose their color, until the calling script is
> fixed to add back in a --color option[1].

I think it's better for the calling script to be fixed to use "git
diff", since it is producing output for the sake of the user instead
of for machine parsing.  That way, the script gets the benefit of
other changes like --decorate automatically.

So I don't see that as a regression.

Where I worry is about commands where the line between porcelain and
plumbing blur, like "git log --format=raw".  I actually still prefer
the approach where "color.ui=always" becomes impossible to express in
config and each command takes a --color option.

If we want to be extra fancy, we could make git take a --color option
instead of requiring each command to do it.

To support existing scripts, we could treat "-c color.ui=always" as a
historical synonym for --color=always, either temporarily or
indefinitely.  Making it clear that this is only there for historical
reasons would make it less likely that other options make the same
mistake in the future.

Thanks,
Jonathan

  reply	other threads:[~2017-10-10 19:03 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
2017-10-10 12:51           ` Junio C Hamano
2017-10-10 13:06             ` Jeff King
2017-10-10 19:03               ` Jonathan Nieder [this message]
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=20171010190314.GW19555@aiede.mtv.corp.google.com \
    --to=jrnieder@gmail.com \
    --cc=ayiehere@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /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).