Skip to content

Commit

Permalink
cli: make crossterm not respect NO_COLOR
Browse files Browse the repository at this point in the history
The `NO_COLOR` spec says that user-specified config is supposed to
override the `$NO_COLOR` environment variable, and we do correctly use
the `ColorFormatter` when `ui.color= "always"` is set in the user's
config. However, it turns out that `NO_COLOR=1` still resulted in no
color because `crossterm` also respects the variable, so the color
codes the `ColorFormatter` requested had no effect. Since `crossterm`
doesn't know about our user configs, it cannot decide whether to
respect `$NO_COLOR`, so let's tell `crossterm` to always use the
colors we tell it to use.
  • Loading branch information
martinvonz committed Dec 7, 2024
1 parent 23f9cc7 commit 6672327
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 3 deletions.
2 changes: 2 additions & 0 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3627,6 +3627,8 @@ impl CliRunner {
.fold(builder, |builder, config| builder.add_source(config))
.build()
.unwrap();
// Tell crossterm to ignore NO_COLOR (we check it ourselves)
crossterm::style::force_color_output(true);
let stacked_config = config_from_environment(config);
let mut ui = Ui::with_config(&stacked_config)
.expect("default config should be valid, env vars are stringly typed");
Expand Down
5 changes: 2 additions & 3 deletions cli/tests/test_global_opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,10 +439,9 @@ fn test_color_config() {
// Test that NO_COLOR does NOT override the request for color in the config file
test_env.add_env_var("NO_COLOR", "1");
let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", "commit_id"]);
// TODO: Should have color
insta::assert_snapshot!(stdout, @r###"
[1m[m@[0m [m230dd059e1b059aefc0da06a2e5a7dbf22362f22[m
[1m[m◆[0m [m0000000000000000000000000000000000000000[m
[1m[38;5;2m@[0m [38;5;4m230dd059e1b059aefc0da06a2e5a7dbf22362f22[39m
[1m[38;5;14m◆[0m [38;5;4m0000000000000000000000000000000000000000[39m
"###);

// Test that per-repo config overrides the user config.
Expand Down

0 comments on commit 6672327

Please sign in to comment.