fix(highlight): increase syntax highlighter config priority

This commit is contained in:
Gabriel Levcovitz 2025-02-19 00:40:18 -03:00
parent 442a2c020d
commit dde7c01d7d
No known key found for this signature in database
2 changed files with 71 additions and 98 deletions

View File

@ -46,9 +46,8 @@ jobs:
with: with:
toolchain: ${{ matrix.rust }} toolchain: ${{ matrix.rust }}
components: clippy components: clippy
# TODO: Uncomment - name: Clippy
# - name: Clippy run: cargo clippy --all -- -D warnings
# run: cargo clippy --all -- -D warnings
- name: Run tests - name: Run tests
if: matrix.rust == 'stable' if: matrix.rust == 'stable'
run: cargo test --all --verbose --features ${{matrix.features}} run: cargo test --all --verbose --features ${{matrix.features}}

View File

@ -1,5 +1,3 @@
use std::fmt;
use crate::highlighters::Highlighter; use crate::highlighters::Highlighter;
use crate::highlighters::MietteHighlighter; use crate::highlighters::MietteHighlighter;
use crate::protocol::Diagnostic; use crate::protocol::Diagnostic;
@ -9,6 +7,8 @@ use crate::NarratableReportHandler;
use crate::ReportHandler; use crate::ReportHandler;
use crate::ThemeCharacters; use crate::ThemeCharacters;
use crate::ThemeStyles; use crate::ThemeStyles;
use cfg_if::cfg_if;
use std::fmt;
/// Settings to control the color format used for graphical rendering. /// Settings to control the color format used for graphical rendering.
#[derive(Copy, Clone, Debug, Eq, PartialEq, Default)] #[derive(Copy, Clone, Debug, Eq, PartialEq, Default)]
@ -99,7 +99,9 @@ impl MietteHandlerOpts {
/// Setting this option will not force color output. In all cases, the /// Setting this option will not force color output. In all cases, the
/// current color configuration via /// current color configuration via
/// [`color()`](MietteHandlerOpts::color()) takes precedence over /// [`color()`](MietteHandlerOpts::color()) takes precedence over
/// highlighter configuration. /// highlighter configuration. However, this option does take precedence over
/// [`rgb_colors()`](MietteHandlerOpts::rgb_colors()) (meaning syntax highlighting will be
/// enabled regardless of the value of [`MietteHandlerOpts::rgb_colors`]).
pub fn with_syntax_highlighting( pub fn with_syntax_highlighting(
mut self, mut self,
highlighter: impl Highlighter + Send + Sync + 'static, highlighter: impl Highlighter + Send + Sync + 'static,
@ -203,6 +205,8 @@ impl MietteHandlerOpts {
/// first place. That is handled by the [`MietteHandlerOpts::color`] /// first place. That is handled by the [`MietteHandlerOpts::color`]
/// setting. If colors are not being used, the value of `rgb_colors` has /// setting. If colors are not being used, the value of `rgb_colors` has
/// no effect. /// no effect.
///
/// It also does not control colors when a syntax highlighter is in use.
pub fn rgb_colors(mut self, color: RgbColors) -> Self { pub fn rgb_colors(mut self, color: RgbColors) -> Self {
self.rgb_colors = color; self.rgb_colors = color;
self self
@ -292,12 +296,8 @@ impl MietteHandlerOpts {
} else { } else {
ThemeStyles::none() ThemeStyles::none()
}; };
let highlighter_opt = HighlighterOption::select( let highlighter_opt =
self.color, HighlighterOption::select(self.color, self.highlighter, syscall::supports_color());
self.rgb_colors,
self.highlighter,
syscall::supports_color(),
);
let theme = self.theme.unwrap_or(GraphicalTheme { characters, styles }); let theme = self.theme.unwrap_or(GraphicalTheme { characters, styles });
let mut handler = GraphicalReportHandler::new_themed(theme) let mut handler = GraphicalReportHandler::new_themed(theme)
.with_width(width) .with_width(width)
@ -419,36 +419,33 @@ enum HighlighterOption {
} }
impl HighlighterOption { impl HighlighterOption {
#[cfg_attr(not(feature = "syntect-highlighter"), allow(unused_variables))]
fn select( fn select(
color: Option<bool>, color: Option<bool>,
rgb_colors: RgbColors,
highlighter: Option<MietteHighlighter>, highlighter: Option<MietteHighlighter>,
supports_color: bool, supports_color: bool,
) -> HighlighterOption { ) -> HighlighterOption {
#[cfg(not(feature = "syntect-highlighter"))] if color == Some(false) || (color == None && !supports_color) {
let opt = highlighter return HighlighterOption::Disable;
}
highlighter
.map(HighlighterOption::EnableCustom) .map(HighlighterOption::EnableCustom)
.unwrap_or(HighlighterOption::Disable); .unwrap_or_default()
#[cfg(feature = "syntect-highlighter")] }
let opt = if color == Some(false) { }
HighlighterOption::Disable
} else if color == Some(true) || supports_color { impl Default for HighlighterOption {
match highlighter { fn default() -> Self {
Some(highlighter) => HighlighterOption::EnableCustom(highlighter), cfg_if! {
None => match rgb_colors { if #[cfg(feature = "syntect-highlighter")] {
// Because the syntect highlighter currently only supports 24-bit truecolor, // Because the syntect highlighter currently only supports 24-bit truecolor,
// respect RgbColor::Never by disabling the highlighter. // it supersedes and ignores the `rgb_colors` config.
// TODO: In the future, find a way to convert the RGB syntect theme // TODO: In the future, if we find a way to convert the RGB syntect theme
// into an ANSI color theme. // into an ANSI color theme, we can take `rgb_colors` into account.
RgbColors::Never => HighlighterOption::Disable, HighlighterOption::EnableSyntect
_ => HighlighterOption::EnableSyntect, } else {
}, HighlighterOption::Disable
} }
} else { }
HighlighterOption::Disable
};
opt
} }
} }
@ -532,13 +529,11 @@ mod tests {
fn test_highlighter_option() { fn test_highlighter_option() {
// Syntax highlighting is enabled depending on several variables: // Syntax highlighting is enabled depending on several variables:
// - The `color` config // - The `color` config
// - The `rgb_colors` config
// - The `highlighter` config // - The `highlighter` config
// - Whether the `syntect-highlighter` feature is enabled // - Whether the `syntect-highlighter` feature is enabled
// - Whether the terminal supports color // - Whether the terminal supports color
// //
// This test asserts the expected selected highlighter depending on some combinations of // This test asserts the expected highlighter depending on combinations of those variables.
// those variables, but it's not comprehensive.
macro_rules! assert_highlighter_opt { macro_rules! assert_highlighter_opt {
(opts = $opts:expr, supports_color = $sup_color:literal, expected = $expected:pat $(,)?) => { (opts = $opts:expr, supports_color = $sup_color:literal, expected = $expected:pat $(,)?) => {
@ -556,12 +551,8 @@ mod tests {
expected_with_syntect = $expected_with:pat, expected_with_syntect = $expected_with:pat,
expected_without_syntect = $expected_without:pat $(,)? expected_without_syntect = $expected_without:pat $(,)?
) => {{ ) => {{
let highlighter_opt = HighlighterOption::select( let highlighter_opt =
$opts.color, HighlighterOption::select($opts.color, $opts.highlighter, $sup_color);
$opts.rgb_colors,
$opts.highlighter,
$sup_color,
);
cfg_if! { cfg_if! {
if #[cfg(feature = "syntect-highlighter")] { if #[cfg(feature = "syntect-highlighter")] {
assert!(matches!(highlighter_opt, $expected_with)); assert!(matches!(highlighter_opt, $expected_with));
@ -572,80 +563,63 @@ mod tests {
}}; }};
} }
// The default case never enables highlighting, even with color support and the // When color is explicitly disabled, highlighting is also always disabled.
// `syntect-highlighter` feature enabled, because `RgbColors::Never` prevents it. assert_highlighter_opt!(
// TODO: This is undesirable, highlighting should be enabled if color is supported and opts = MietteHandlerOpts::new().color(false),
// `syntect-highlighter` is enabled, regardless of the RGB config. supports_color = true,
expected = HighlighterOption::Disable,
);
// When color is unset and the terminal doesn't support color, highlighting is disabled.
assert_highlighter_opt!( assert_highlighter_opt!(
opts = MietteHandlerOpts::new(), opts = MietteHandlerOpts::new(),
supports_color = false, supports_color = false,
expected = HighlighterOption::Disable, expected = HighlighterOption::Disable,
); );
assert_highlighter_opt!(
opts = MietteHandlerOpts::new().rgb_colors(RgbColors::Never), // With explicit or implicit color support, highlighting is automatically enabled when
supports_color = false, // `syntect-highlighter` is enabled.
expected = HighlighterOption::Disable,
);
assert_highlighter_opt!( assert_highlighter_opt!(
opts = MietteHandlerOpts::new().color(true), opts = MietteHandlerOpts::new().color(true),
supports_color = true, supports_color = false,
expected = HighlighterOption::Disable, expected_with_syntect = HighlighterOption::EnableSyntect,
expected_without_syntect = HighlighterOption::Disable,
); );
assert_highlighter_opt!(
opts = MietteHandlerOpts::new(),
supports_color = true,
expected_with_syntect = HighlighterOption::EnableSyntect,
expected_without_syntect = HighlighterOption::Disable,
);
// With explicit or implicit color support, if custom highlighting is set, it's enabled.
assert_highlighter_opt!(
opts = MietteHandlerOpts::new()
.color(true)
.with_syntax_highlighting(BlankHighlighter),
supports_color = false,
expected = HighlighterOption::EnableCustom(_),
);
assert_highlighter_opt!(
opts = MietteHandlerOpts::new().with_syntax_highlighting(BlankHighlighter),
supports_color = true,
expected = HighlighterOption::EnableCustom(_),
);
// Setting `RgbColors::Never` has no effect when syntax highlighting is enabled.
assert_highlighter_opt!( assert_highlighter_opt!(
opts = MietteHandlerOpts::new() opts = MietteHandlerOpts::new()
.color(true) .color(true)
.rgb_colors(RgbColors::Never), .rgb_colors(RgbColors::Never),
supports_color = true,
expected = HighlighterOption::Disable,
);
// With explicit or implicit color support, and explicit RGB support, highlighting is
// enabled when `syntect-highlighter` is enabled.
assert_highlighter_opt!(
opts = MietteHandlerOpts::new()
.color(true)
.rgb_colors(RgbColors::Preferred),
supports_color = false, supports_color = false,
expected_with_syntect = HighlighterOption::EnableSyntect, expected_with_syntect = HighlighterOption::EnableSyntect,
expected_without_syntect = HighlighterOption::Disable, expected_without_syntect = HighlighterOption::Disable,
); );
assert_highlighter_opt!( assert_highlighter_opt!(
opts = MietteHandlerOpts::new() opts = MietteHandlerOpts::new().rgb_colors(RgbColors::Never),
.color(true)
.rgb_colors(RgbColors::Always),
supports_color = false,
expected_with_syntect = HighlighterOption::EnableSyntect,
expected_without_syntect = HighlighterOption::Disable,
);
assert_highlighter_opt!(
opts = MietteHandlerOpts::new().rgb_colors(RgbColors::Preferred),
supports_color = true, supports_color = true,
expected_with_syntect = HighlighterOption::EnableSyntect, expected_with_syntect = HighlighterOption::EnableSyntect,
expected_without_syntect = HighlighterOption::Disable, expected_without_syntect = HighlighterOption::Disable,
); );
assert_highlighter_opt!(
opts = MietteHandlerOpts::new().rgb_colors(RgbColors::Always),
supports_color = true,
expected_with_syntect = HighlighterOption::EnableSyntect,
expected_without_syntect = HighlighterOption::Disable,
);
// Custom highlighter is enabled when color is enabled, regardless of RGB support.
assert_highlighter_opt!(
opts = MietteHandlerOpts::new()
.color(true)
.with_syntax_highlighting(BlankHighlighter),
supports_color = true,
expected = HighlighterOption::EnableCustom(_),
);
assert_highlighter_opt!(
opts = MietteHandlerOpts::new()
.color(false)
.with_syntax_highlighting(BlankHighlighter),
supports_color = true,
expected_with_syntect = HighlighterOption::Disable,
// TODO: This is incorrect, it should be disabled because color is disabled.
expected_without_syntect = HighlighterOption::EnableCustom(_),
);
} }
} }