Skip to content

Commit c954302

Browse files
committed
Fix ANSI colour support
The version of curses that is used on Unix like systems does not properly support truecolor. Because of this, existing colour codes indexes were being overwritten to provide support for arbitrary custom colours. This worked fine in the terminal emulators tested on Linux, as the colours were reset back to their originals when the tool exited. However, on MacOS, the colour changes would remain after the tool exited, until the terminal was restarted. To fix this, in terminals that do not support overwriting colours, a provided colour triplet will be matched against the closest ANSI indexed colour.
1 parent cd7052f commit c954302

11 files changed

Lines changed: 642 additions & 96 deletions

File tree

.github/workflows/pull-request.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ jobs:
1010
- name: Install dependencies
1111
run: sudo apt-get install build-essential libncursesw5-dev pkg-config liblzma-dev
1212
- name: Test
13-
run: cargo test
13+
run: cargo test -- --test-threads=1
1414
- name: Build
1515
run: cargo build --release
1616
- name: Test Run
@@ -30,7 +30,7 @@ jobs:
3030
with:
3131
toolchain: stable
3232
- name: Test
33-
run: cargo test
33+
run: cargo test -- --test-threads=1
3434
- name: Build
3535
run: cargo build --release
3636
- name: Test Run
@@ -41,8 +41,8 @@ jobs:
4141
steps:
4242
- uses: actions/checkout@v1
4343
- name: Test
44-
run: cargo test --target x86_64-pc-windows-msvc --release
44+
run: cargo test --target x86_64-pc-windows-msvc --release -- --test-threads=1
4545
- name: Build
4646
run: cargo build --target x86_64-pc-windows-msvc --release
4747
- name: Test Run
48-
run: cargo run --target x86_64-pc-windows-msvc --release -- --version
48+
run: cargo run --target x86_64-pc-windows-msvc --release -- --version

appveyor.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ install:
2323

2424
test_script:
2525
- if [%APPVEYOR_REPO_TAG%]==[false] (
26+
cargo test --target x86_64-pc-windows-msvc -- --test-threads=1 &&
2627
cargo run --target x86_64-pc-windows-msvc --release -- --version
2728
)
2829

scripts/format.bash

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,4 @@ rust_version="nightly-2019-09-13"
88

99
rustup update "$rust_version"
1010
rustup component add rustfmt --toolchain "$rust_version"
11-
cargo +"$rust_version" fmt --all -- --check
11+
cargo +"$rust_version" fmt --all -- --check

scripts/test.bash

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,5 @@ set -e
44
set -u
55
set -o pipefail
66

7-
cargo test
8-
cargo test --release
7+
cargo test -- --test-threads=1
8+
cargo test --release -- --test-threads=1

src/config/mod.rs

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ mod utils;
44
use crate::config::theme::Theme;
55
use crate::config::utils::{editor_from_env, get_bool, get_color, get_input, get_string, open_git_config};
66
use crate::display::color::Color;
7-
use std::convert::TryFrom;
87

98
#[derive(Clone, Debug)]
109
pub(crate) struct Config {
@@ -45,25 +44,29 @@ impl Config {
4544
let git_config = open_git_config()?;
4645
Ok(Config {
4746
theme: Theme {
48-
color_foreground: get_color(&git_config, "interactive-rebase-tool.foregroundColor", Color::White)?,
47+
color_foreground: get_color(&git_config, "interactive-rebase-tool.foregroundColor", Color::Default)?,
4948
color_background: get_color(&git_config, "interactive-rebase-tool.backgroundColor", Color::Default)?,
5049
color_selected_background: get_color(
5150
&git_config,
5251
"interactive-rebase-tool.selectedBackgroundColor",
53-
Color::try_from("35,35,40").unwrap(),
52+
Color::Index(237),
5453
)?,
55-
color_indicator: get_color(&git_config, "interactive-rebase-tool.indicatorColor", Color::Cyan)?,
56-
color_action_break: get_color(&git_config, "interactive-rebase-tool.breakColor", Color::White)?,
57-
color_action_drop: get_color(&git_config, "interactive-rebase-tool.dropColor", Color::Red)?,
58-
color_action_edit: get_color(&git_config, "interactive-rebase-tool.editColor", Color::Blue)?,
59-
color_action_exec: get_color(&git_config, "interactive-rebase-tool.execColor", Color::White)?,
60-
color_action_fixup: get_color(&git_config, "interactive-rebase-tool.fixupColor", Color::Magenta)?,
61-
color_action_pick: get_color(&git_config, "interactive-rebase-tool.pickColor", Color::Green)?,
62-
color_action_reword: get_color(&git_config, "interactive-rebase-tool.rewordColor", Color::Yellow)?,
63-
color_action_squash: get_color(&git_config, "interactive-rebase-tool.squashColor", Color::Cyan)?,
64-
color_diff_add: get_color(&git_config, "interactive-rebase-tool.diffAddColor", Color::Green)?,
65-
color_diff_change: get_color(&git_config, "interactive-rebase-tool.diffChangeColor", Color::Yellow)?,
66-
color_diff_remove: get_color(&git_config, "interactive-rebase-tool.diffRemoveColor", Color::Red)?,
54+
color_indicator: get_color(&git_config, "interactive-rebase-tool.indicatorColor", Color::LightCyan)?,
55+
color_action_break: get_color(&git_config, "interactive-rebase-tool.breakColor", Color::LightWhite)?,
56+
color_action_drop: get_color(&git_config, "interactive-rebase-tool.dropColor", Color::LightRed)?,
57+
color_action_edit: get_color(&git_config, "interactive-rebase-tool.editColor", Color::LightBlue)?,
58+
color_action_exec: get_color(&git_config, "interactive-rebase-tool.execColor", Color::LightWhite)?,
59+
color_action_fixup: get_color(&git_config, "interactive-rebase-tool.fixupColor", Color::LightMagenta)?,
60+
color_action_pick: get_color(&git_config, "interactive-rebase-tool.pickColor", Color::LightGreen)?,
61+
color_action_reword: get_color(&git_config, "interactive-rebase-tool.rewordColor", Color::LightYellow)?,
62+
color_action_squash: get_color(&git_config, "interactive-rebase-tool.squashColor", Color::LightCyan)?,
63+
color_diff_add: get_color(&git_config, "interactive-rebase-tool.diffAddColor", Color::LightGreen)?,
64+
color_diff_change: get_color(
65+
&git_config,
66+
"interactive-rebase-tool.diffChangeColor",
67+
Color::LightYellow,
68+
)?,
69+
color_diff_remove: get_color(&git_config, "interactive-rebase-tool.diffRemoveColor", Color::LightRed)?,
6770
character_vertical_spacing: get_string(
6871
&git_config,
6972
"interactive-rebase-tool.verticalSpacingCharacter",

src/display/color.rs

Lines changed: 178 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,24 @@ use std::convert::TryFrom;
22

33
#[derive(Clone, Copy, Debug, PartialEq)]
44
pub(crate) enum Color {
5-
White,
6-
Black,
7-
Blue,
8-
Cyan,
9-
Green,
10-
Magenta,
11-
Red,
12-
Yellow,
5+
LightWhite,
6+
LightBlack,
7+
LightBlue,
8+
LightCyan,
9+
LightGreen,
10+
LightMagenta,
11+
LightRed,
12+
LightYellow,
13+
DarkWhite,
14+
DarkBlack,
15+
DarkBlue,
16+
DarkCyan,
17+
DarkGreen,
18+
DarkMagenta,
19+
DarkRed,
20+
DarkYellow,
1321
Default,
22+
Index(i16),
1423
RGB { red: i16, green: i16, blue: i16 },
1524
}
1625

@@ -19,34 +28,54 @@ impl TryFrom<&str> for Color {
1928

2029
fn try_from(s: &str) -> Result<Self, Self::Error> {
2130
match s {
22-
"black" => Ok(Color::Black),
23-
"blue" => Ok(Color::Blue),
24-
"cyan" => Ok(Color::Cyan),
25-
"green" => Ok(Color::Green),
26-
"magenta" => Ok(Color::Magenta),
27-
"red" => Ok(Color::Red),
28-
"white" => Ok(Color::White),
29-
"yellow" => Ok(Color::Yellow),
31+
"black" => Ok(Color::LightBlack),
32+
"blue" => Ok(Color::LightBlue),
33+
"cyan" => Ok(Color::LightCyan),
34+
"green" => Ok(Color::LightGreen),
35+
"magenta" => Ok(Color::LightMagenta),
36+
"red" => Ok(Color::LightRed),
37+
"white" => Ok(Color::LightWhite),
38+
"yellow" => Ok(Color::LightYellow),
39+
"light black" => Ok(Color::LightBlack),
40+
"light blue" => Ok(Color::LightBlue),
41+
"light cyan" => Ok(Color::LightCyan),
42+
"light green" => Ok(Color::LightGreen),
43+
"light magenta" => Ok(Color::LightMagenta),
44+
"light red" => Ok(Color::LightRed),
45+
"light white" => Ok(Color::LightWhite),
46+
"light yellow" => Ok(Color::LightYellow),
47+
"dark black" => Ok(Color::DarkBlack),
48+
"dark blue" => Ok(Color::DarkBlue),
49+
"dark cyan" => Ok(Color::DarkCyan),
50+
"dark green" => Ok(Color::DarkGreen),
51+
"dark magenta" => Ok(Color::DarkMagenta),
52+
"dark red" => Ok(Color::DarkRed),
53+
"dark white" => Ok(Color::DarkWhite),
54+
"dark yellow" => Ok(Color::DarkYellow),
3055
"transparent" | "-1" => Ok(Color::Default),
3156
_ => {
3257
let matches: Vec<&str> = s.split(',').collect();
3358

34-
if matches.len() == 3 {
35-
let red = matches.get(0).unwrap().parse::<i16>().unwrap_or(-1);
36-
let green = matches.get(1).unwrap().parse::<i16>().unwrap_or(-1);
37-
let blue = matches.get(2).unwrap().parse::<i16>().unwrap_or(-1);
38-
39-
if red > -1 && green > -1 && blue > -1 && red < 256 && green < 256 && blue < 256 {
40-
// values need to be mapped to 0-1000
41-
return Ok(Color::RGB {
42-
red: ((f64::from(red) / 255.0) * 1000.0) as i16,
43-
green: ((f64::from(green) / 255.0) * 1000.0) as i16,
44-
blue: ((f64::from(blue) / 255.0) * 1000.0) as i16,
45-
});
46-
}
47-
return Err(format!("Invalid color string: {}. Values must be within 0-255.", s));
59+
match matches.len() {
60+
1 => {
61+
let color_index = s.parse::<i16>();
62+
match color_index {
63+
Ok(i) if i >= 0 && i < 256 => Ok(Color::Index(i)),
64+
_ => Err(format!("Invalid color value: {}", s)),
65+
}
66+
},
67+
3 => {
68+
let red = matches.get(0).unwrap().parse::<i16>().unwrap_or(-1);
69+
let green = matches.get(1).unwrap().parse::<i16>().unwrap_or(-1);
70+
let blue = matches.get(2).unwrap().parse::<i16>().unwrap_or(-1);
71+
72+
if red > -1 && green > -1 && blue > -1 && red < 256 && green < 256 && blue < 256 {
73+
return Ok(Color::RGB { red, green, blue });
74+
}
75+
Err(format!("Invalid color string: {}. Values must be within 0-255.", s))
76+
},
77+
_ => Err(format!("Invalid color value: {}", s)),
4878
}
49-
Err(format!("Invalid color string: {}", s))
5079
},
5180
}
5281
}
@@ -59,42 +88,131 @@ mod tests {
5988

6089
#[test]
6190
fn action_try_from_str_black() {
62-
assert_eq!(Color::try_from("black").unwrap(), Color::Black);
91+
assert_eq!(Color::try_from("black").unwrap(), Color::LightBlack);
92+
}
93+
94+
#[test]
95+
fn action_try_from_str_light_black() {
96+
assert_eq!(Color::try_from("light black").unwrap(), Color::LightBlack);
97+
}
98+
99+
#[test]
100+
fn action_try_from_str_dark_black() {
101+
assert_eq!(Color::try_from("dark black").unwrap(), Color::DarkBlack);
63102
}
64103

65104
#[test]
66105
fn action_try_from_str_blue() {
67-
assert_eq!(Color::try_from("blue").unwrap(), Color::Blue);
106+
assert_eq!(Color::try_from("blue").unwrap(), Color::LightBlue);
107+
}
108+
109+
#[test]
110+
fn action_try_from_str_light_blue() {
111+
assert_eq!(Color::try_from("light blue").unwrap(), Color::LightBlue);
112+
}
113+
114+
#[test]
115+
fn action_try_from_str_dark_blue() {
116+
assert_eq!(Color::try_from("dark blue").unwrap(), Color::DarkBlue);
68117
}
69118

70119
#[test]
71120
fn action_try_from_str_cyan() {
72-
assert_eq!(Color::try_from("cyan").unwrap(), Color::Cyan);
121+
assert_eq!(Color::try_from("cyan").unwrap(), Color::LightCyan);
122+
}
123+
124+
#[test]
125+
fn action_try_from_str_light_cyan() {
126+
assert_eq!(Color::try_from("light cyan").unwrap(), Color::LightCyan);
127+
}
128+
129+
#[test]
130+
fn action_try_from_str_dark_cyan() {
131+
assert_eq!(Color::try_from("dark cyan").unwrap(), Color::DarkCyan);
73132
}
74133

75134
#[test]
76135
fn action_try_from_str_green() {
77-
assert_eq!(Color::try_from("green").unwrap(), Color::Green);
136+
assert_eq!(Color::try_from("green").unwrap(), Color::LightGreen);
137+
}
138+
139+
#[test]
140+
fn action_try_from_str_light_green() {
141+
assert_eq!(Color::try_from("light green").unwrap(), Color::LightGreen);
142+
}
143+
144+
#[test]
145+
fn action_try_from_str_dark_green() {
146+
assert_eq!(Color::try_from("dark green").unwrap(), Color::DarkGreen);
78147
}
79148

80149
#[test]
81150
fn action_try_from_str_magenta() {
82-
assert_eq!(Color::try_from("magenta").unwrap(), Color::Magenta);
151+
assert_eq!(Color::try_from("magenta").unwrap(), Color::LightMagenta);
152+
}
153+
154+
#[test]
155+
fn action_try_from_str_light_magenta() {
156+
assert_eq!(Color::try_from("light magenta").unwrap(), Color::LightMagenta);
157+
}
158+
159+
#[test]
160+
fn action_try_from_str_dark_magenta() {
161+
assert_eq!(Color::try_from("dark magenta").unwrap(), Color::DarkMagenta);
83162
}
84163

85164
#[test]
86165
fn action_try_from_str_red() {
87-
assert_eq!(Color::try_from("red").unwrap(), Color::Red);
166+
assert_eq!(Color::try_from("red").unwrap(), Color::LightRed);
167+
}
168+
169+
#[test]
170+
fn action_try_from_str_light_red() {
171+
assert_eq!(Color::try_from("light red").unwrap(), Color::LightRed);
172+
}
173+
174+
#[test]
175+
fn action_try_from_str_dark_red() {
176+
assert_eq!(Color::try_from("dark red").unwrap(), Color::DarkRed);
88177
}
89178

90179
#[test]
91180
fn action_try_from_str_white() {
92-
assert_eq!(Color::try_from("white").unwrap(), Color::White);
181+
assert_eq!(Color::try_from("white").unwrap(), Color::LightWhite);
93182
}
94183

95184
#[test]
96185
fn action_try_from_str_yellow() {
97-
assert_eq!(Color::try_from("yellow").unwrap(), Color::Yellow);
186+
assert_eq!(Color::try_from("yellow").unwrap(), Color::LightYellow);
187+
}
188+
189+
#[test]
190+
fn action_try_from_str_light_yellow() {
191+
assert_eq!(Color::try_from("light yellow").unwrap(), Color::LightYellow);
192+
}
193+
194+
#[test]
195+
fn action_try_from_str_dark_yellow() {
196+
assert_eq!(Color::try_from("dark yellow").unwrap(), Color::DarkYellow);
197+
}
198+
199+
#[test]
200+
fn action_try_from_color_index_minimum() {
201+
assert_eq!(Color::try_from("0").unwrap(), Color::Index(0));
202+
}
203+
204+
#[test]
205+
fn action_try_from_color_index_maximum() {
206+
assert_eq!(Color::try_from("255").unwrap(), Color::Index(255));
207+
}
208+
209+
#[test]
210+
fn action_try_from_color_rgb_color() {
211+
assert_eq!(Color::try_from("100,101,102").unwrap(), Color::RGB {
212+
red: 100,
213+
green: 101,
214+
blue: 102
215+
});
98216
}
99217

100218
#[test]
@@ -170,7 +288,26 @@ mod tests {
170288
}
171289

172290
#[test]
173-
fn action_try_from_str_invalid() {
174-
assert_eq!(Color::try_from("invalid").unwrap_err(), "Invalid color string: invalid");
291+
fn action_try_from_color_index_invalid_upper_limit() {
292+
assert_eq!(Color::try_from("256").unwrap_err(), "Invalid color value: 256");
293+
}
294+
295+
#[test]
296+
fn action_try_from_color_index_invalid_lower_limit() {
297+
// -1 is transparent/default and a valid value
298+
assert_eq!(Color::try_from("-2").unwrap_err(), "Invalid color value: -2");
299+
}
300+
301+
#[test]
302+
fn action_try_from_str_invalid_single_value() {
303+
assert_eq!(Color::try_from("invalid").unwrap_err(), "Invalid color value: invalid");
304+
}
305+
306+
#[test]
307+
fn action_try_from_str_invalid_multiple_value() {
308+
assert_eq!(
309+
Color::try_from("invalid,invalid").unwrap_err(),
310+
"Invalid color value: invalid,invalid"
311+
);
175312
}
176313
}

0 commit comments

Comments
 (0)