diff options
author | Dan Davison <dandavison7@gmail.com> | 2020-05-20 22:46:08 -0400 |
---|---|---|
committer | Dan Davison <dandavison7@gmail.com> | 2020-05-22 22:45:37 -0400 |
commit | 2245582fbef18d94b0bef2fec3389f710dadc8d9 (patch) | |
tree | c3b70c9ec42e4ce95e0e26f807278eb636e70987 | |
parent | 291d2fb7fa8cfc2a0886af895972326d00fb4dbe (diff) |
Implement hunk styles using style string arguments
- Do not apply foreground syntax style if it is "null syntect style"
This isn't really correct. We should find either a valid sentinel
value, or a way to only do the superimposing when we're doing
syntax-highlighting.
- Add --zero-style option (style for unchanged hunk lines)
- Implement --color-only using an option rewrite rule
-rw-r--r-- | src/cli.rs | 163 | ||||
-rw-r--r-- | src/config.rs | 174 | ||||
-rw-r--r-- | src/delta.rs | 38 | ||||
-rw-r--r-- | src/paint.rs | 117 | ||||
-rw-r--r-- | src/style.rs | 20 | ||||
-rw-r--r-- | src/tests/ansi_test_utils.rs | 80 | ||||
-rw-r--r-- | src/tests/integration_test_utils.rs | 21 | ||||
-rw-r--r-- | src/tests/mod.rs | 1 | ||||
-rw-r--r-- | src/tests/test_hunk_highlighting.rs | 171 |
9 files changed, 299 insertions, 486 deletions
@@ -71,37 +71,41 @@ pub struct Opt { #[structopt(long = "dark")] pub dark: bool, - #[structopt(long = "minus-style")] + #[structopt(long = "minus-style", default_value = "normal auto")] /// The style for removed lines. - pub minus_style: Option<String>, + pub minus_style: String, - #[structopt(long = "minus-emph-style")] + #[structopt(long = "minus-emph-style", default_value = "normal auto")] /// The style for emphasized sections of removed lines. - pub minus_emph_style: Option<String>, + pub minus_emph_style: String, - #[structopt(long = "plus-style")] + #[structopt(long = "zero-style", default_value = "syntax normal")] + /// The style for unchanged lines. + pub zero_style: String, + + #[structopt(long = "plus-style", default_value = "syntax auto")] /// The style for removed lines. - pub plus_style: Option<String>, + pub plus_style: String, - #[structopt(long = "plus-emph-style")] + #[structopt(long = "plus-emph-style", default_value = "syntax auto")] /// The style for emphasized sections of removed lines. - pub plus_emph_style: Option<String>, + pub plus_emph_style: String, #[structopt(long = "minus-color")] /// The background color for removed lines. - pub _deprecated_minus_color: Option<String>, + pub deprecated_minus_background_color: Option<String>, #[structopt(long = "minus-emph-color")] /// The background color for emphasized sections of removed lines. - pub _deprecated_minus_emph_color: Option<String>, + pub deprecated_minus_emph_background_color: Option<String>, #[structopt(long = "plus-color")] /// The background color for added lines. - pub _deprecated_plus_color: Option<String>, + pub deprecated_plus_background_color: Option<String>, #[structopt(long = "plus-emph-color")] /// The background color for emphasized sections of added lines. - pub _deprecated_plus_emph_color: Option<String>, + pub deprecated_plus_emph_background_color: Option<String>, #[structopt(long = "theme", env = "BAT_THEME")] /// The code syntax highlighting theme to use. Use --theme=none to disable syntax highlighting. @@ -113,7 +117,7 @@ pub struct Opt { #[structopt(long = "highlight-removed")] /// DEPRECATED: supply 'syntax' as the foreground color in --minus-style. - pub highlight_minus_lines: bool, + pub deprecated_highlight_minus_lines: bool, #[structopt(long = "color-only")] /// Do not alter the input in any way other than applying colors. Equivalent to @@ -246,8 +250,9 @@ pub fn process_command_line_arguments<'a>(mut opt: Opt) -> config::Config<'a> { _check_validity(&opt, &assets); - _apply_rewrite_rules(&mut opt); - + // Apply rewrite rules + _rewrite_style_strings_to_honor_deprecated_options(&mut opt); + _rewrite_options_to_implement_color_only(&mut opt); // We do not use the full width, in case `less --status-column` is in effect. See #41 and #10. // TODO: There seems to be some confusion in the accounting: we are actually leaving 2 // characters unused for less at the right edge of the terminal, despite the subtraction of 1 @@ -321,38 +326,88 @@ fn _check_validity(opt: &Opt, assets: &HighlightingAssets) { } } -fn _apply_rewrite_rules(opt: &mut Opt) { - opt.minus_style = _make_style_string( - opt.minus_style.as_deref(), - opt._deprecated_minus_color.as_deref(), +/// Implement --color-only +fn _rewrite_options_to_implement_color_only(opt: &mut Opt) { + if opt.color_only { + opt.keep_plus_minus_markers = true; + opt.tab_width = 0; + opt.commit_style = SectionStyle::Plain; + opt.file_style = SectionStyle::Plain; + opt.hunk_style = SectionStyle::Plain; + } +} + +/// Honor deprecated arguments by rewriting the canonical --*-style arguments if appropriate. +// TODO: How to avoid repeating the default values for style options here and in +// the structopt definition? +// If --highlight-removed was passed then we should set minus and minus emph foreground to "syntax", if they are still at their default values. +fn _rewrite_style_strings_to_honor_deprecated_options(opt: &mut Opt) { + let deprecated_minus_foreground_arg = if opt.deprecated_highlight_minus_lines { + Some("syntax") + } else { + None + }; + + if let Some(rewritten) = _rewrite_style_string_maybe( + &opt.minus_style, + ("normal", "auto"), + ( + deprecated_minus_foreground_arg, + opt.deprecated_minus_background_color.as_deref(), + ), "minus", - ); - opt.minus_emph_style = _make_style_string( - opt.minus_emph_style.as_deref(), - opt._deprecated_minus_emph_color.as_deref(), + ) { + opt.minus_style = rewritten.to_string(); + } + if let Some(rewritten) = _rewrite_style_string_maybe( + &opt.minus_emph_style, + ("normal", "auto"), + ( + deprecated_minus_foreground_arg, + opt.deprecated_minus_emph_background_color.as_deref(), + ), "minus-emph", - ); - opt.plus_style = _make_style_string( - opt.plus_style.as_deref(), - opt._deprecated_plus_color.as_deref(), + ) { + opt.minus_emph_style = rewritten.to_string(); + } + if let Some(rewritten) = _rewrite_style_string_maybe( + &opt.plus_style, + ("syntax", "auto"), + (None, opt.deprecated_plus_background_color.as_deref()), "plus", - ); - opt.plus_emph_style = _make_style_string( - opt.plus_emph_style.as_deref(), - opt._deprecated_plus_emph_color.as_deref(), + ) { + opt.plus_style = rewritten.to_string(); + } + if let Some(rewritten) = _rewrite_style_string_maybe( + &opt.plus_emph_style, + ("syntax", "auto"), + (None, opt.deprecated_plus_emph_background_color.as_deref()), "plus-emph", - ); + ) { + opt.plus_emph_style = rewritten.to_string(); + } } -pub fn _make_style_string( - style: Option<&str>, - background_color: Option<&str>, +pub fn _rewrite_style_string_maybe( + style: &str, + style_default_pair: (&str, &str), + deprecated_args_style_pair: (Option<&str>, Option<&str>), element_name: &str, ) -> Option<String> { - match (style, background_color) { - (_, None) => style.map(str::to_string), - (None, Some(background_color)) => Some(format!("syntax {}", background_color)), - (Some(_), Some(_)) => { + let format_style = |pair: (&str, &str)| format!("{} {}", pair.0, pair.1); + match (style, deprecated_args_style_pair) { + (_, (None, None)) => None, // no rewrite + (style, deprecated_args_style_pair) if style == format_style(style_default_pair) => { + // TODO: We allow the deprecated argument values to have effect if + // the style argument value is equal to its default value. This is + // non-ideal, because the user may have explicitly supplied the + // style argument (i.e. it might just happen to equal the default). + Some(format_style(( + deprecated_args_style_pair.0.unwrap_or(style_default_pair.0), + deprecated_args_style_pair.1.unwrap_or(style_default_pair.1), + ))) + } + (_, (_, Some(_))) => { eprintln!( "--{name}-color cannot be used with --{name}-style. \ Use --{name}-style=\"fg bg attr1 attr2 ...\" to set \ @@ -363,6 +418,16 @@ pub fn _make_style_string( ); process::exit(1); } + (_, (Some(_), None)) => { + eprintln!( + "Deprecated option --highlight-removed cannot be used with \ + --{name}-style. Use --{name}-style=\"fg bg attr1 attr2 ...\" \ + to set foreground color, background color, and style \ + attributes.", + name = element_name, + ); + process::exit(1); + } } } @@ -473,19 +538,31 @@ mod tests { } assert_eq!( config.minus_style.background.unwrap(), - style::get_minus_color_default(expected_mode == Mode::Light, is_true_color) + style::get_minus_background_color_default( + expected_mode == Mode::Light, + is_true_color + ) ); assert_eq!( config.minus_emph_style.background.unwrap(), - style::get_minus_emph_color_default(expected_mode == Mode::Light, is_true_color) + style::get_minus_emph_background_color_default( + expected_mode == Mode::Light, + is_true_color + ) ); assert_eq!( config.plus_style.background.unwrap(), - style::get_plus_color_default(expected_mode == Mode::Light, is_true_color) + style::get_plus_background_color_default( + expected_mode == Mode::Light, + is_true_color + ) ); assert_eq!( config.plus_emph_style.background.unwrap(), - style::get_plus_emph_color_default(expected_mode == Mode::Light, is_true_color) + style::get_plus_emph_background_color_default( + expected_mode == Mode::Light, + is_true_color + ) ); } } diff --git a/src/config.rs b/src/config.rs index 7af41290..04286733 100644 --- a/src/config.rs +++ b/src/config.rs @@ -10,7 +10,6 @@ use syntect::parsing::SyntaxSet; use crate::bat::output::PagingMode; use crate::bat::terminal::to_ansi_color; use crate::cli; -use crate::delta::State; use crate::env; use crate::style; use crate::syntect_color; @@ -23,6 +22,7 @@ pub struct Config<'a> { pub max_line_distance_for_naively_paired_lines: f64, pub minus_style: Style, pub minus_emph_style: Style, + pub zero_style: Style, pub plus_style: Style, pub plus_emph_style: Style, pub minus_line_marker: &'a str, @@ -44,41 +44,6 @@ pub struct Config<'a> { pub paging_mode: PagingMode, } -#[allow(dead_code)] -pub enum ColorLayer { - Background, - Foreground, -} -use ColorLayer::*; -use State::*; - -impl<'a> Config<'a> { - pub fn get_style(&self, state: &State) -> Option<Style> { - match state { - HunkMinus => Some(self.minus_style), - HunkZero => None, - HunkPlus => Some(self.plus_style), - _ => panic!("Invalid"), - } - } - - #[allow(dead_code)] - pub fn get_color(&self, state: &State, layer: ColorLayer) -> Option<Color> { - match (self.get_style(state), layer) { - (Some(style), Background) => style.background, - (Some(style), Foreground) => style.foreground, - (None, _) => None, - } - } - - pub fn should_syntax_highlight(&self, state: &State) -> bool { - match self.get_style(state) { - Some(style) => style.foreground == Some(style::SYNTAX_HIGHLIGHTING_COLOR), - None => false, - } - } -} - pub fn get_config<'a>( opt: cli::Opt, syntax_set: SyntaxSet, @@ -87,24 +52,7 @@ pub fn get_config<'a>( terminal_width: usize, paging_mode: PagingMode, ) -> Config<'a> { - // Implement --color-only - let keep_plus_minus_markers = if opt.color_only { - true - } else { - opt.keep_plus_minus_markers - }; let background_color_extends_to_terminal_width = opt.width != Some("variable".to_string()); - let tab_width = if opt.color_only { 0 } else { opt.tab_width }; - let (commit_style, file_style, hunk_style) = if opt.color_only { - ( - cli::SectionStyle::Plain, - cli::SectionStyle::Plain, - cli::SectionStyle::Plain, - ) - } else { - (opt.commit_style, opt.file_style, opt.hunk_style) - }; - let theme_name_from_bat_pager = env::get_env_var("BAT_THEME"); let (is_light_mode, theme_name) = get_is_light_mode_and_theme_name( opt.theme.as_ref(), @@ -113,7 +61,7 @@ pub fn get_config<'a>( &theme_set, ); - let (minus_style, minus_emph_style, plus_style, plus_emph_style) = + let (minus_style, minus_emph_style, zero_style, plus_style, plus_emph_style) = make_styles(&opt, is_light_mode, true_color); let theme = if style::is_no_syntax_highlighting_theme_name(&theme_name) { @@ -123,8 +71,16 @@ pub fn get_config<'a>( }; let dummy_theme = theme_set.themes.values().next().unwrap().clone(); - let minus_line_marker = if keep_plus_minus_markers { "-" } else { " " }; - let plus_line_marker = if keep_plus_minus_markers { "+" } else { " " }; + let minus_line_marker = if opt.keep_plus_minus_markers { + "-" + } else { + " " + }; + let plus_line_marker = if opt.keep_plus_minus_markers { + "+" + } else { + " " + }; let max_line_distance_for_naively_paired_lines = env::get_env_var("DELTA_EXPERIMENTAL_MAX_LINE_DISTANCE_FOR_NAIVELY_PAIRED_LINES") @@ -139,20 +95,21 @@ pub fn get_config<'a>( max_line_distance_for_naively_paired_lines, minus_style, minus_emph_style, + zero_style, plus_style, plus_emph_style, minus_line_marker, plus_line_marker, - commit_style, + commit_style: opt.commit_style, commit_color: color_from_rgb_or_ansi_code(&opt.commit_color, true_color), - file_style, + file_style: opt.file_style, file_color: color_from_rgb_or_ansi_code(&opt.file_color, true_color), - hunk_style, + hunk_style: opt.hunk_style, hunk_color: color_from_rgb_or_ansi_code(&opt.hunk_color, true_color), true_color, terminal_width, background_color_extends_to_terminal_width, - tab_width, + tab_width: opt.tab_width, syntax_set, null_style: Style::new(), null_syntect_style: SyntectStyle::default(), @@ -220,71 +177,66 @@ fn make_styles<'a>( opt: &'a cli::Opt, is_light_mode: bool, true_color: bool, -) -> (Style, Style, Style, Style) { - let minus_style = make_style( - opt.minus_style.as_deref(), - Some(style::get_minus_color_default(is_light_mode, true_color)), +) -> (Style, Style, Style, Style, Style) { + let minus_style = parse_style_string( + &opt.minus_style, None, + Some(style::get_minus_background_color_default( + is_light_mode, + true_color, + )), true_color, ); - let minus_emph_style = make_style( - opt.minus_emph_style.as_deref(), - Some(style::get_minus_emph_color_default( + let minus_emph_style = parse_style_string( + &opt.minus_emph_style, + None, + Some(style::get_minus_emph_background_color_default( is_light_mode, true_color, )), - minus_style.foreground, true_color, ); - let plus_style = make_style( - opt.plus_style.as_deref(), - Some(style::get_plus_color_default(is_light_mode, true_color)), + let zero_style = parse_style_string(&opt.zero_style, None, None, true_color); + + let plus_style = parse_style_string( + &opt.plus_style, None, + Some(style::get_plus_background_color_default( + is_light_mode, + true_color, + )), true_color, ); - let plus_emph_style = make_style( - opt.plus_emph_style.as_deref(), - Some(style::get_plus_emph_color_default( + let plus_emph_style = parse_style_string( + &opt.plus_emph_style, + None, + Some(style::get_plus_emph_background_color_default( is_light_mode, true_color, )), - plus_style.foreground, true_color, ); - (minus_style, minus_emph_style, plus_style, plus_emph_style) + ( + minus_style, + minus_emph_style, + zero_style, + plus_style, + plus_emph_style, + ) } /// Construct ansi_term Style from style string supplied on command line, /// together with defaults. -fn make_style( - style_string: Option<&str>, - background_default: Option<Color>, - foreground_default: Option<Color>, - true_color: bool, -) -> Style { - if let Some(s) = style_string { - parse_style_string(s, background_default, foreground_default, true_color) - } else { - Style { - foreground: foreground_default, - background: background_default, - ..Style::new() - } - } -} - -fn parse_style_string( +pub fn parse_style_string( style_string: &str, - background_default: Option<Color>, foreground_default: Option<Color>, + background_default: Option<Color>, true_color: bool, ) -> Style { - let mut foreground = foreground_default; - let mut background = background_default; let mut style = Style::new(); let mut seen_foreground = false; let mut seen_background = false; @@ -306,10 +258,12 @@ fn parse_style_string( } else if s == "underline" { style.is_underline = true; } else if !seen_foreground { - foreground = color_from_rgb_or_ansi_code_with_default(Some(s), None, true_color); + style.foreground = + color_from_rgb_or_ansi_code_with_default(s, foreground_default, true_color); seen_foreground = true; } else if !seen_background { - background = color_from_rgb_or_ansi_code_with_default(Some(s), None, true_color); + style.background = + color_from_rgb_or_ansi_code_with_default(s, background_default, true_color); seen_background = true; } else { eprintln!( @@ -325,11 +279,7 @@ fn parse_style_string( process::exit(1); } } - Style { - foreground, - background, - ..style - } + style } fn color_from_rgb_or_ansi_code(s: &str, true_color: bool) -> Color { @@ -350,14 +300,18 @@ fn color_from_rgb_or_ansi_code(s: &str, true_color: bool) -> Color { } fn color_from_rgb_or_ansi_code_with_default( - arg: Option<&str>, + arg: &str, default: Option<Color>, true_color: bool, ) -> Option<Color> { - match arg.map(str::to_lowercase) { - Some(s) if s == "none" => None, - Some(s) if s == "syntax" => Some(style::SYNTAX_HIGHLIGHTING_COLOR), - Some(s) => Some(color_from_rgb_or_ansi_code(&s, true_color)), - None => default, + let arg = arg.to_lowercase(); + if arg == "normal" { + None + } else if arg == "auto" { + default + } else if arg == "syntax" { + Some(style::SYNTAX_HIGHLIGHTING_COLOR) + } else { + Some(color_from_rgb_or_ansi_code(&arg, true_color)) } } diff --git a/src/delta.rs b/src/delta.rs index 3483259a..65eb1b27 100644 --- a/src/delta.rs +++ b/src/delta.rs @@ -253,17 +253,17 @@ fn handle_hunk_meta_line( cli::SectionStyle::Omit => return Ok(()), }; let (raw_code_fragment, line_number) = parse::parse_hunk_metadata(&line); - let code_fragment = prepare(raw_code_fragment, false, config); - if !code_fragment.is_empty() { - let syntax_style_sections = Painter::get_line_syntax_style_sections( - &code_fragment, - true, + let lines = vec![prepare(raw_code_fragment, false, config)]; + if !lines[0].is_empty() { + let syntax_style_sections = Painter::get_syntax_style_sections_for_lines( + &lines, + &State::HunkMeta, &mut painter.highlighter, &painter.config, ); Painter::paint_lines( - vec![syntax_style_sections], - vec![vec![(Style::new(), &code_fragment)]], + syntax_style_sections, + vec![vec![(Style::new(), lines[0].as_str())]], &mut painter.output_buffer, config, "", @@ -321,26 +321,22 @@ fn handle_hunk_line( let state = State::HunkZero; let prefix = if line.is_empty() { "" } else { &line[..1] }; painter.paint_buffered_lines(); - let line = prepare(&line, true, config); - let syntax_style_sections = if config.should_syntax_highlight(&state) { - Painter::get_line_syntax_style_sections( - &line, - true, - &mut painter.highlighter, - &painter.config, - ) - } else { - vec![(config.null_syntect_style, line.as_str())] - }; - let diff_style_sections = vec![(Style::new(), line.as_str())]; + let lines = vec![prepare(&line, true, config)]; + let syntax_style_sections = Painter::get_syntax_style_sections_for_lines( + &lines, + &state, + &mut painter.highlighter, + &painter.config, + ); + let diff_style_sections = vec![(config.zero_style, lines[0].as_str())]; Painter::paint_lines( - vec![syntax_style_sections], + syntax_style_sections, vec![diff_style_sections], &mut painter.output_buffer, config, prefix, - config.null_style, + config.zero_style, None, ); state diff --git a/src/paint.rs b/src/paint.rs index 9ef25eca..3b947318 100644 --- a/src/paint.rs +++ b/src/paint.rs @@ -9,6 +9,7 @@ use crate::config; use crate::delta::State; use crate::edits; use crate::paint::superimpose_style_sections::superimpose_style_sections; +use crate::style::SyntaxHighlightable; pub const ANSI_CSI_ERASE_IN_LINE: &str = "\x1b[K"; pub const ANSI_SGR_RESET: &str = "\x1b[0m"; @@ -58,13 +59,13 @@ impl<'a> Painter<'a> { pub fn paint_buffered_lines(&mut self) { let minus_line_syntax_style_sections = Self::get_syntax_style_sections_for_lines( &self.minus_lines, - self.config.should_syntax_highlight(&State::HunkMinus), + &State::HunkMinus, &mut self.highlighter, self.config, ); let plus_line_syntax_style_sections = Self::get_syntax_style_sections_for_lines( &self.plus_lines, - self.config.should_syntax_highlight(&State::HunkPlus), + &State::HunkPlus, &mut self.highlighter, self.config, ); @@ -105,7 +106,12 @@ impl<'a> Painter<'a> { output_buffer: &mut String, config: &config::Config, prefix: &str, - background_style: Style, + // TODO: When we have distinct minus_style and minus_non_emph_style, + // this function will have to watch the emph-types encountered in the + // line to determine whether the appropriate default style is + // minus_style (no emph section encountered) or minus_emph_style + // (otherwise). + default_style: Style, background_color_extends_to_terminal_width: Option<bool>, ) { for (syntax_sections, diff_sections) in @@ -113,12 +119,15 @@ impl<'a> Painter<'a> { { let mut ansi_strings = Vec::new(); if prefix != "" { - ansi_strings.push(background_style.paint(prefix)); + ansi_strings.push(default_style.paint(prefix)); } let mut dropped_prefix = prefix == ""; // TODO: Hack - for (style, mut text) in - superimpose_style_sections(syntax_sections, diff_sections, config.true_color) - { + for (style, mut text) in superimpose_style_sections( + syntax_sections, + diff_sections, + config.true_color, + config.null_syntect_style, + ) { if !dropped_prefix { if text.len() > 0 { text.remove(0); @@ -127,7 +136,7 @@ impl<'a> Painter<'a> { } ansi_strings.push(style.paint(text)); } - ansi_strings.push(background_style.paint("")); + ansi_strings.push(default_style.paint("")); let line = &mut ansi_term::ANSIStrings(&ansi_strings).to_string(); let background_color_extends_to_terminal_width = match background_color_extends_to_terminal_width { @@ -159,41 +168,50 @@ impl<'a> Painter<'a> { Ok(()) } - fn get_syntax_style_sections_for_lines<'s>( - lines: &'s [String], - should_syntax_highlight: bool, + pub fn should_compute_syntax_highlighting(state: &State, config: &config::Config) -> bool { + if config.theme.is_none() { + return false; + } + match state { + State::HunkMinus => { + config.minus_style.is_syntax_highlighted() + || config.minus_emph_style.is_syntax_highlighted() + } + State::HunkZero => config.zero_style.is_syntax_highlighted(), + State::HunkPlus => { + config.plus_style.is_syntax_highlighted() + || config.plus_emph_style.is_syntax_highlighted() + } + State::HunkMeta => true, + _ => panic!( + "should_compute_syntax_highlighting is undefined for state {:?}", + state + ), + } + } + + pub fn get_syntax_style_sections_for_lines<'s>( + lines: &'s Vec<String>, + state: &State, highlighter: &mut HighlightLines, config: &config::Config, ) -> Vec<Vec<(SyntectStyle, &'s str)>> { + let fake = !Painter::should_compute_syntax_highlighting(state, config); let mut line_sections = Vec::new(); for line in lines.iter() { - line_sections.push(Painter::get_line_syntax_style_sections( - line, - should_syntax_highlight, - highlighter, - &config, - )); + if fake { + line_sections.push(vec![(config.null_syntect_style, line.as_str())]) + } else { + line_sections.push(highlighter.highlight(line, &config.syntax_set)) + } } line_sections } - pub fn get_line_syntax_style_sections( - line: &'a str, - should_syntax_highlight: bool, - highlighter: &mut HighlightLines, - config: &config::Config, - ) -> Vec<(SyntectStyle, &'a str)> { - if should_syntax_highlight && config.theme.is_some() { |