This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2079

color: honor NO_COLOR
Needs RevisionPublic

Authored by indygreg on Feb 7 2018, 12:17 PM.

Details

Reviewers
lothiraldan
yuja
Group Reviewers
hg-reviewers
Summary

The http://no-color.org/ initiative is trying to get programs that
emit color by default to honor a NO_COLOR environment variable to
disable color.

I think that's a good idea. So this commit implements support for
NO_COLOR.

I'm not sure if the precedence of settings is proper here. Right now,
NO_COLOR overrides config settings set by hgrc or --config. But it
doesn't override --color. I can see an argument for honoring
--config as well. Same for hgrc (since color is enabled by default
these days). But the existing logic/precedence is unclear to me. So
I went with an easy implementation.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg created this revision.Feb 7 2018, 12:17 PM
lothiraldan accepted this revision.Feb 7 2018, 12:35 PM
lothiraldan added a subscriber: lothiraldan.

👍 Hurrah for standards!

quark added a subscriber: quark.Feb 7 2018, 3:41 PM

You might want to let run-tests.py drop NO_COLOR for tests.

yuja added a subscriber: yuja.Feb 10 2018, 7:23 AM

I'm not sure if the precedence of settings is proper here.

Perhaps it should be placed at the same level as $EDITOR (i.e. envrcitems)?

And can you update help/color and environment?

martinvonz added a subscriber: martinvonz.Feb 10 2018, 11:26 AM
In D2079#35154, @yuja wrote:

I'm not sure if the precedence of settings is proper here.

Perhaps it should be placed at the same level as $EDITOR (i.e. envrcitems)?

I haven't bothered checking what that means for precedence, but I have an argument for giving NO_COLOR higher precedence than at least the system config: we set the system config for all our users at Google and some may want to override with NO_COLOR. I know someone (Jun? Yuya?) did some work related to precedence of EDITOR a while ago, but I don't remember what the goal of that was.

quark added a subscriber: durin42.Feb 10 2018, 2:13 PM

The goal was to allow users to override system default using environment variables. So a separate [systemdefaults] (proposed by @durin42) becomes unnecessary.

yuja requested changes to this revision.Feb 15 2018, 7:22 AM

(just clarify the current state of this patch)

This revision now requires changes to proceed.Feb 15 2018, 7:22 AM

Revision Contents
Changeset List

PathPackages
Mmercurial/color.py (7 lines)
Mtests/test-status-color.t (36 lines)

Diff 5298

mercurial/color.py

def _modesetup(ui): def _modesetup(ui):
if ui.plain('color'): if ui.plain('color'):
return None return None
config = ui.config('ui', 'color') config = ui.config('ui', 'color')
if config == 'debug': if config == 'debug':
return 'debug' return 'debug'
# The http://no-color.org/ initiative wants to standardize on an environment
# variable to disable color. The value of this variable doesn't matter.
if 'NO_COLOR' in encoding.environ:
# Allow --color CLI argument to override NO_COLOR
if ui.configsource('ui', 'color') != '--color':
return None
auto = (config == 'auto') auto = (config == 'auto')
always = False always = False
if not auto and util.parsebool(config): if not auto and util.parsebool(config):
# We want the config to behave like a boolean, "on" is actually auto, # We want the config to behave like a boolean, "on" is actually auto,
# but "always" value is treated as a special case to reduce confusion. # but "always" value is treated as a special case to reduce confusion.
if ui.configsource('ui', 'color') == '--color' or config == 'always': if ui.configsource('ui', 'color') == '--color' or config == 'always':
always = True always = True
else: else:

tests/test-status-color.t

$ HGPLAINEXCEPT=color hg status --color=debug $ HGPLAINEXCEPT=color hg status --color=debug
[status.unknown|? ][status.unknown|a/1/in_a_1] (glob) [status.unknown|? ][status.unknown|a/1/in_a_1] (glob)
[status.unknown|? ][status.unknown|a/in_a] (glob) [status.unknown|? ][status.unknown|a/in_a] (glob)
[status.unknown|? ][status.unknown|b/1/in_b_1] (glob) [status.unknown|? ][status.unknown|b/1/in_b_1] (glob)
[status.unknown|? ][status.unknown|b/2/in_b_2] (glob) [status.unknown|? ][status.unknown|b/2/in_b_2] (glob)
[status.unknown|? ][status.unknown|b/in_b] (glob) [status.unknown|? ][status.unknown|b/in_b] (glob)
[status.unknown|? ][status.unknown|in_root] [status.unknown|? ][status.unknown|in_root]
NO_COLOR disables color
$ NO_COLOR=1 hg status
? a/1/in_a_1
? a/in_a
? b/1/in_b_1
? b/2/in_b_2
? b/in_b
? in_root
$ NO_COLOR=0 hg status
? a/1/in_a_1
? a/in_a
? b/1/in_b_1
? b/2/in_b_2
? b/in_b
? in_root
$ NO_COLOR= hg status
? a/1/in_a_1
? a/in_a
? b/1/in_b_1
? b/2/in_b_2
? b/in_b
? in_root
NO_COLOR is overridden by --color
$ NO_COLOR=1 hg --color=always status
\x1b[0;35;1;4m? \x1b[0m\x1b[0;35;1;4ma/1/in_a_1\x1b[0m (esc)
\x1b[0;35;1;4m? \x1b[0m\x1b[0;35;1;4ma/in_a\x1b[0m (esc)
\x1b[0;35;1;4m? \x1b[0m\x1b[0;35;1;4mb/1/in_b_1\x1b[0m (esc)
\x1b[0;35;1;4m? \x1b[0m\x1b[0;35;1;4mb/2/in_b_2\x1b[0m (esc)
\x1b[0;35;1;4m? \x1b[0m\x1b[0;35;1;4mb/in_b\x1b[0m (esc)
\x1b[0;35;1;4m? \x1b[0m\x1b[0;35;1;4min_root\x1b[0m (esc)
hg status with template hg status with template
$ hg status -T "{label('red', path)}\n" --color=debug $ hg status -T "{label('red', path)}\n" --color=debug
[red|a/1/in_a_1] [red|a/1/in_a_1]
[red|a/in_a] [red|a/in_a]
[red|b/1/in_b_1] [red|b/1/in_b_1]
[red|b/2/in_b_2] [red|b/2/in_b_2]
[red|b/in_b] [red|b/in_b]
[red|in_root] [red|in_root]