diff options
author | Dan Davison <dandavison7@gmail.com> | 2020-05-09 12:32:01 -0400 |
---|---|---|
committer | Dan Davison <dandavison7@gmail.com> | 2020-05-16 09:04:38 -0400 |
commit | 79979c272d6daa876ffb3dbad7a6550c22155c8c (patch) | |
tree | 30baf82bd2892ffb73bfd3f6dab6ce21b6ae6ebd /src | |
parent | 21537d6eb8f07d6c1e71c757fb8f0fd583449219 (diff) |
Hunk highlighting test and refactoring
- New command-line option --syntax highlight to select which category
of hunk lines to be syntax highlighted. --highlight-removed is now
deprecated.
- New hunk highlighting test tests syntax highlighting and background colors
- Refactor: construction of style modifiers for config struct
- Refactor: compute minus and plus foreground style sections separately
- Refactor: foreground style logic
- Refactor: rearrange style modifier construction for config
- Refactor: change variable name: => highlight_minus_lines
- Refactor: clean up --color-only config implementation
Diffstat (limited to 'src')
-rw-r--r-- | src/cli.rs | 191 | ||||
-rw-r--r-- | src/config.rs | 214 | ||||
-rw-r--r-- | src/delta.rs | 778 | ||||
-rw-r--r-- | src/main.rs | 1 | ||||
-rw-r--r-- | src/paint.rs | 95 | ||||
-rw-r--r-- | src/tests/ansi_test_utils.rs | 111 | ||||
-rw-r--r-- | src/tests/integration_test_utils.rs | 68 | ||||
-rw-r--r-- | src/tests/mod.rs | 4 | ||||
-rw-r--r-- | src/tests/test_example_diffs.rs | 575 | ||||
-rw-r--r-- | src/tests/test_hunk_highlighting.rs | 160 |
10 files changed, 1338 insertions, 859 deletions
@@ -2,12 +2,14 @@ use std::process; use std::str::FromStr; use std::string::ToString; +use bit_set::BitSet; use console::Term; use structopt::StructOpt; use crate::bat::assets::HighlightingAssets; use crate::bat::output::PagingMode; use crate::config; +use crate::delta::State; use crate::env; use crate::style; @@ -68,21 +70,37 @@ pub struct Opt { pub dark: bool, #[structopt(long = "minus-color")] - /// The background color to use for removed lines. + /// The background color for removed lines. pub minus_color: Option<String>, #[structopt(long = "minus-emph-color")] - /// The background color to use for emphasized sections of removed lines. + /// The background color for emphasized sections of removed lines. pub minus_emph_color: Option<String>, + #[structopt(long = "minus-foreground-color")] + /// The foreground color for removed lines. + pub minus_foreground_color: Option<String>, + + #[structopt(long = "minus-emph-foreground-color")] + /// The foreground color for emphasized sections of removed lines. + pub minus_emph_foreground_color: Option<String>, + #[structopt(long = "plus-color")] - /// The background color to use for added lines. + /// The background color for added lines. pub plus_color: Option<String>, #[structopt(long = "plus-emph-color")] - /// The background color to use for emphasized sections of added lines. + /// The background color for emphasized sections of added lines. pub plus_emph_color: Option<String>, + #[structopt(long = "plus-foreground-color")] + /// Disable syntax highlighting and instead use this foreground color for added lines. + pub plus_foreground_color: Option<String>, + + #[structopt(long = "plus-emph-foreground-color")] + /// Disable syntax highlighting and instead use this foreground color for emphasized sections of added lines. + pub plus_emph_foreground_color: Option<String>, + #[structopt(long = "theme", env = "BAT_THEME")] /// The code syntax highlighting theme to use. Use --theme=none to disable syntax highlighting. /// If the theme is not set using this option, it will be taken from the BAT_THEME environment @@ -91,10 +109,15 @@ pub struct Opt { /// --file-color, --hunk-color to configure the colors of other parts of the diff output. pub theme: Option<String>, + /// A string consisting only of the characters '-', '0', '+', specifying + /// which of the 3 diff hunk line-types (removed, unchanged, added) should + /// be syntax-highlighted. "all" and "none" are also valid values. + #[structopt(long = "syntax-highlight", default_value = "0+")] + pub lines_to_be_syntax_highlighted: String, + #[structopt(long = "highlight-removed")] - /// Apply syntax highlighting to removed lines. The default is to - /// apply syntax highlighting to unchanged and new lines only. - pub highlight_removed: bool, + /// DEPRECATED: use --syntax-highlight. + pub highlight_minus_lines: bool, #[structopt(long = "color-only")] /// Do not alter the input in any way other than applying colors. Equivalent to @@ -296,6 +319,7 @@ pub fn process_command_line_arguments<'a>( true_color, available_terminal_width, paging_mode, + get_lines_to_be_syntax_highlighted(opt), ) } @@ -304,3 +328,156 @@ fn is_truecolor_terminal() -> bool { .map(|colorterm| colorterm == "truecolor" || colorterm == "24bit") .unwrap_or(false) } + +fn get_lines_to_be_syntax_highlighted(opt: &Opt) -> BitSet { + if opt.highlight_minus_lines { + eprintln!("--highlight-removed is deprecated: use --syntax-highlight."); + } + + let syntax_highlight_lines = match opt.lines_to_be_syntax_highlighted.to_lowercase().as_ref() { + "none" => "", + // This is the default value of the new option: honor the deprecated option if it has been used. + "0+" => match opt.highlight_minus_lines { + true => "-0+", + false => "0+", + }, + "all" => "-0+", + s => s, + } + .to_string(); + + let mut lines_to_be_syntax_highlighted = BitSet::new(); + for line_type in syntax_highlight_lines.chars() { + lines_to_be_syntax_highlighted.insert(match line_type { + '-' => State::HunkMinus as usize, + '0' => State::HunkZero as usize, + '+' => State::HunkPlus as usize, + s => { + eprintln!("Invalid --syntax-highlight value: {}. Valid characters are \"-\", \"0\", \"+\".", s); + process::exit(1); + } + }); + } + lines_to_be_syntax_highlighted +} + +#[cfg(test)] +mod tests { + use std::env; + + use crate::bat::assets::HighlightingAssets; + use crate::cli; + use crate::style; + use crate::tests::integration_test_utils::integration_test_utils; + + #[test] + fn test_theme_selection() { + #[derive(PartialEq)] + enum Mode { + Light, + Dark, + }; + let assets = HighlightingAssets::new(); + for ( + theme_option, + bat_theme_env_var, + mode_option, // (--light, --dark) + expected_theme, + expected_mode, + ) in vec![ + (None, "", None, style::DEFAULT_DARK_THEME, Mode::Dark), + (Some("GitHub".to_string()), "", None, "GitHub", Mode::Light), + ( + Some("GitHub".to_string()), + "1337", + None, + "GitHub", + Mode::Light, + ), + (None, "1337", None, "1337", Mode::Dark), + ( + None, + "<not set>", + None, + style::DEFAULT_DARK_THEME, + Mode::Dark, + ), + ( + None, + "", + Some(Mode::Light), + style::DEFAULT_LIGHT_THEME, + Mode::Light, + ), + ( + None, + "", + Some(Mode::Dark), + style::DEFAULT_DARK_THEME, + Mode::Dark, + ), + ( + None, + "<@@@@@>", + Some(Mode::Light), + style::DEFAULT_LIGHT_THEME, + Mode::Light, + ), + (None, "1337", Some(Mode::Light), "1337", Mode::Light), + (Some("none".to_string()), "", None, "none", Mode::Dark), + ( + Some("None".to_string()), + "", + Some(Mode::Light), + "None", + Mode::Light, + ), + ] { + if bat_theme_env_var == "<not set>" { + env::remove_var("BAT_THEME") + } else { + env::set_var("BAT_THEME", bat_theme_env_var); + } + let is_true_color = true; + let mut options = integration_test_utils::get_command_line_options(); + options.theme = theme_option; + match mode_option { + Some(Mode::Light) => { + options.light = true; + options.dark = false; + } + Some(Mode::Dark) => { + options.light = false; + options.dark = true; + } + None => { + options.light = false; + options.dark = false; + } + } + let config = cli::process_command_line_arguments(&assets, &options); + assert_eq!(config.theme_name, expected_theme); + if style::is_no_syntax_highlighting_theme_name(expected_theme) { + assert!(config.theme.is_none()) + } else { + assert_eq!(config.theme.unwrap().name.as_ref().unwrap(), expected_theme); + } + assert_eq!( + config.minus_style_modifier.background.unwrap(), + style::get_minus_color_default(expected_mode == Mode::Light, is_true_color) + ); + assert_eq!( + config.minus_emph_style_modifier.background.unwrap(), + style::get_minus_emph_color_default(expected_mode == Mode::Light, is_true_color) + ); + assert_eq!( + config.plus_style_modifier.background.unwrap(), + style::get_plus_color_default(expected_mode == Mode::Light, is_true_color) + ); + assert_eq!( + config.plus_emph_style_modifier.background.unwrap(), + style::get_plus_emph_color_default(expected_mode == Mode::Light, is_true_color) + ); + } + } +} diff --git a/src/config.rs b/src/config.rs index 9fc757d3..f9e42196 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,11 +1,13 @@ use std::process; use std::str::FromStr; +use bit_set::BitSet; use syntect::highlighting::{Color, Style, StyleModifier, Theme, ThemeSet}; use syntect::parsing::SyntaxSet; use crate::bat::output::PagingMode; use crate::cli; +use crate::delta::State; use crate::env; use crate::paint; use crate::style; @@ -15,13 +17,15 @@ pub struct Config<'a> { pub theme_name: String, pub max_line_distance: f64, pub max_line_distance_for_naively_paired_lines: f64, + pub minus_foreground_style_modifier: Option<StyleModifier>, pub minus_style_modifier: StyleModifier, pub minus_emph_style_modifier: StyleModifier, + pub plus_foreground_style_modifier: Option<StyleModifier>, pub plus_style_modifier: StyleModifier, pub plus_emph_style_modifier: StyleModifier, pub minus_line_marker: &'a str, pub plus_line_marker: &'a str, - pub highlight_removed: bool, + pub lines_to_be_syntax_highlighted: BitSet, pub commit_style: cli::SectionStyle, pub commit_color: Color, pub file_style: cli::SectionStyle, @@ -38,6 +42,37 @@ pub struct Config<'a> { pub paging_mode: PagingMode, } +#[allow(dead_code)] +pub enum ColorLayer { + Background, + Foreground, +} +use ColorLayer::*; +use State::*; + +impl<'a> Config<'a> { + #[allow(dead_code)] + pub fn get_color(&self, state: &State, layer: ColorLayer) -> Option<Color> { + let modifier = match state { + HunkMinus => Some(self.minus_style_modifier), + HunkZero => None, + HunkPlus => Some(self.plus_style_modifier), + _ => panic!("Invalid"), + }; + match (modifier, layer) { + (Some(modifier), Background) => modifier.background, + (Some(modifier), Foreground) => modifier.foreground, + (None, _) => None, + } + } + + #[allow(dead_code)] + pub fn should_syntax_highlight(&self, state: &State) -> bool { + self.lines_to_be_syntax_highlighted + .contains((*state).clone() as usize) + } +} + pub fn get_config<'a>( opt: &'a cli::Opt, syntax_set: &'a SyntaxSet, @@ -45,6 +80,7 @@ pub fn get_config<'a>( true_color: bool, terminal_width: usize, paging_mode: PagingMode, + lines_to_be_syntax_highlighted: BitSet, ) -> Config<'a> { // Implement --color-only let keep_plus_minus_markers = if opt.color_only { @@ -54,20 +90,14 @@ pub fn get_config<'a>( }; 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 = if opt.color_only { - cli::SectionStyle::Plain - } else { - opt.commit_style - }; - let file_style = if opt.color_only { - cli::SectionStyle::Plain + let (commit_style, file_style, hunk_style) = if opt.color_only { + ( + cli::SectionStyle::Plain, + cli::SectionStyle::Plain, + cli::SectionStyle::Plain, + ) } else { - opt.file_style - }; - let hunk_style = if opt.color_only { - cli::SectionStyle::Plain - } else { - opt.hunk_style + (opt.commit_style, opt.file_style, opt.hunk_style) }; let theme_name_from_bat_pager = env::get_env_var("BAT_THEME"); @@ -78,56 +108,26 @@ pub fn get_config<'a>( theme_set, ); + let ( + minus_style_modifier, + plus_style_modifier, + minus_emph_style_modifier, + plus_emph_style_modifier, + minus_foreground_style_modifier, + plus_foreground_style_modifier, + ) = make_style_modifiers( + &lines_to_be_syntax_highlighted, + opt, + is_light_mode, + true_color, + ); + let theme = if style::is_no_syntax_highlighting_theme_name(&theme_name) { None } else { Some(&theme_set.themes[&theme_name]) }; - let minus_style_modifier = StyleModifier { - background: Some(color_from_rgb_or_ansi_code_with_default( - opt.minus_color.as_ref(), - style::get_minus_color_default(is_light_mode, true_color), - )), - foreground: if opt.highlight_removed { - None - } else { - Some(style::NO_COLOR) - }, - font_style: None, - }; - - let minus_emph_style_modifier = StyleModifier { - background: Some(color_from_rgb_or_ansi_code_with_default( - opt.minus_emph_color.as_ref(), - style::get_minus_emph_color_default(is_light_mode, true_color), - )), - foreground: if opt.highlight_removed { - None - } else { - Some(style::NO_COLOR) - }, - font_style: None, - }; - - let plus_style_modifier = StyleModifier { - background: Some(color_from_rgb_or_ansi_code_with_default( - opt.plus_color.as_ref(), - style::get_plus_color_default(is_light_mode, true_color), - )), - foreground: None, - font_style: None, - }; - - let plus_emph_style_modifier = StyleModifier { - background: Some(color_from_rgb_or_ansi_code_with_default( - opt.plus_emph_color.as_ref(), - style::get_plus_emph_color_default(is_light_mode, true_color), - )), - foreground: None, - font_style: None, - }; - let minus_line_marker = if keep_plus_minus_markers { "-" } else { " " }; let plus_line_marker = if keep_plus_minus_markers { "+" } else { " " }; @@ -141,11 +141,13 @@ pub fn get_config<'a>( theme_name, max_line_distance: opt.max_line_distance, max_line_distance_for_naively_paired_lines, + minus_foreground_style_modifier, minus_style_modifier, minus_emph_style_modifier, + plus_foreground_style_modifier, plus_style_modifier, plus_emph_style_modifier, - highlight_removed: opt.highlight_removed, + lines_to_be_syntax_highlighted, minus_line_marker, plus_line_marker, commit_style, @@ -220,6 +222,100 @@ fn valid_theme_name_or_none(theme_name: Option<&String>, theme_set: &ThemeSet) - } } +fn make_style_modifiers<'a>( + lines_to_be_syntax_highlighted: &BitSet, + opt: &'a cli::Opt, + is_light_mode: bool, + true_color: bool, +) -> ( + StyleModifier, + StyleModifier, + StyleModifier, + StyleModifier, + Option<StyleModifier>, + Option<StyleModifier>, +) { + // Background styles + let minus_background_style_modifier = StyleModifier { + background: Some(color_from_rgb_or_ansi_code_with_default( + opt.minus_color.as_ref(), + style::get_minus_color_default(is_light_mode, true_color), + )), + foreground: if lines_to_be_syntax_highlighted.contains(State::HunkMinus as usize) { + None + } else { + Some(style::NO_COLOR) + }, + font_style: None, + }; + + let plus_background_style_modifier = StyleModifier { + background: Some(color_from_rgb_or_ansi_code_with_default( + opt.plus_color.as_ref(), + style::get_plus_color_default(is_light_mode, true_color), + )), + foreground: None, + font_style: None, + }; + + // Background emph styles + let minus_background_emph_style_modifier = StyleModifier { + background: Some(color_from_rgb_or_ansi_code_with_default( + opt.minus_emph_color.as_ref(), + style::get_minus_emph_color_default(is_light_mode, true_color), + )), + foreground: if lines_to_be_syntax_highlighted.contains(State::HunkMinus as usize) { + None + } else { + Some(style::NO_COLOR) + }, + font_style: None, + }; + + let plus_background_emph_style_modifier = StyleModifier { + background: Some(color_from_rgb_or_ansi_code_with_default( + opt.plus_emph_color.as_ref(), + style::get_plus_emph_color_default(is_light_mode, true_color), + )), + foreground: None, + font_style: None, + }; + + // Foreground styles (these replace syntax highlighting). + let minus_foreground_style_modifier = match opt.minus_foreground_color.is_some() { + true => Some(StyleModifier { + background: None, + foreground: Some(color_from_rgb_or_ansi_code_with_default( + opt.minus_foreground_color.as_ref(), + style::get_minus_emph_color_default(is_light_mode, true_color), + )), + font_style: None, + }), + false => None, + }; + + let plus_foreground_style_modifier = match opt.plus_foreground_color.is_some() { + true => Some(StyleModifier { + background: None, + foreground: Some(color_from_rgb_or_ansi_code_with_default( + opt.plus_foreground_color.as_ref(), + style::get_plus_emph_color_default(is_light_mode, true_color), + )), + font_style: None, + }), + false => None, + }; + + ( + minus_background_style_modifier, + plus_background_style_modifier, + minus_background_emph_style_modifier, + plus_background_emph_style_modifier, + minus_foreground_style_modifier, + plus_foreground_style_modifier, + ) +} + fn color_from_rgb_or_ansi_code(s: &str) -> Color { let die = || { eprintln!("Invalid color: {}", s); diff --git a/src/delta.rs b/src/delta.rs index 05fc0760..bcbf3e07 100644 --- a/src/delta.rs +++ b/src/delta.rs @@ -13,7 +13,7 @@ use crate::paint::{self, Painter}; use crate::parse; use crate::style; -#[derive(Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq)] pub enum State { CommitMeta, // In commit metadata section FileMeta, // In diff metadata section, between (possible) commit metadata and first hunk @@ -262,8 +262,8 @@ fn handle_hunk_meta_line( let syntax_style_sections = Painter::get_line_syntax_style_sections( &code_fragment, &mut painter.highlighter, + None, &painter.config, - true, ); Painter::paint_lines( vec![syntax_style_sections], @@ -330,25 +330,33 @@ fn handle_hunk_line( State::HunkPlus } Some(' ') => { + 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 = Painter::get_line_syntax_style_sections( - &line, - &mut painter.highlighter, - &painter.config, - true, - ); + let syntax_style_sections = if config.should_syntax_highlight(&state) { + Painter::get_line_syntax_style_sections( + &line, + &mut painter.highlighter, + None, + &painter.config, + ) + } else { + vec![(style::get_no_style(), line.as_str())] + }; + let diff_style_sections = + vec![(style::NO_BACKGROUND_COLOR_STYLE_MODIFIER, line.as_str())]; + Painter::paint_lines( vec![syntax_style_sections], - vec![vec![(style::NO_BACKGROUND_COLOR_STYLE_MODIFIER, &line)]], + vec![diff_style_sections], &mut painter.output_buffer, config, prefix, style::NO_BACKGROUND_COLOR_STYLE_MODIFIER, None, ); - State::HunkZero + state } _ => { // The first character here could be e.g. '\' from '\ No newline at end of file'. This @@ -399,753 +407,3 @@ where line.collect::<String>() } } - -#[cfg(test)] -mod tests { - use super::*; - use console::strip_ansi_codes; - use std::env; - use std::io::BufReader; - use syntect::highlighting::StyleModifier; - - use crate::paint; - - #[test] - fn test_added_file() { - let options = get_command_line_options(); - let output = strip_ansi_codes(&run_delta(ADDED_FILE_INPUT, &options)).to_string(); - assert!(output.contains("\nadded: a.py\n")); - if false { - // TODO: hline width - assert_eq!(output, ADDED_FILE_EXPECTED_OUTPUT); - } - } - - #[test] - #[ignore] // #128 - fn test_added_empty_file() { - let options = get_command_line_options(); - let output = strip_ansi_codes(&run_delta(ADDED_EMPTY_FILE, &options)).to_string(); - assert!(output.contains("\nadded: file\n")); - } - - #[test] - fn test_added_file_directory_path_containing_space() { - let options = get_command_line_options(); - let output = strip_ansi_codes(&run_delta( - ADDED_FILES_DIRECTORY_PATH_CONTAINING_SPACE, - &options, - )) - .to_string(); - assert!(output.contains("\nadded: with space/file1\n")); - assert!(output.contains("\nadded: nospace/file2\n")); - } - - #[test] - fn test_renamed_file() { - let options = get_command_line_options(); - let output = strip_ansi_codes(&run_delta(RENAMED_FILE_INPUT, &options)).to_string(); - assert!(output.contains("\nrenamed: a.py ⟶ b.py\n")); - } - - #[test] - fn test_recognized_file_type() { - // In addition to the background color, the code has language syntax highlighting. - let options = get_command_line_options(); - let input = ADDED_FILE_INPUT; - let output = get_line_of_code_from_delta(&input, &options); - assert_has_color_other_than_plus_color(&output, &options); - } - - #[test] - fn test_unrecognized_file_type_with_theme() { - // In addition to the background color, the code has the foreground color using the default - // .txt syntax under the theme. - let options = get_command_line_options(); - let input = ADDED_FILE_INPUT.replace("a.py", "a"); - let output = get_line_of_code_from_delta(&input, &options); - assert_has_color_other_than_plus_color(&output, &options); - } - - #[test] - fn test_unrecognized_file_type_no_theme() { - // The code has the background color only. (Since there is no theme, the code has no - // foreground ansi color codes.) - let mut options = get_command_line_options(); - options.theme = Some("none".to_string()); - let input = ADDED_FILE_INPUT.replace("a.py", "a"); - let output = get_line_of_code_from_delta(&input, &options); - assert_has_plus_color_only(&output, &options); - } - - #[test] - fn test_theme_selection() { - #[derive(PartialEq)] - enum Mode { - Light, - Dark, - }; - let assets = HighlightingAssets::new(); - for ( - theme_option, - bat_theme_env_var, - mode_option, // (--light, --dark) - expected_theme, - expected_mode, - ) in vec![ - (None, "", None, style::DEFAULT_DARK_THEME, Mode::Dark), - (Some("GitHub".to_string()), "", None, "GitHub", Mode::Light), - ( - Some("GitHub".to_string()), - "1337", - None, - "GitHub", - Mode::Light, - ), - (None, "1337", None, "1337", Mode::Dark), - ( - None, - "<not set>", - None, - style::DEFAULT_DARK_THEME, - Mode::Dark, - ), - ( - None, - "", - Some(Mode::Light), - style::DEFAULT_LIGHT_THEME, - Mode::Light, - ), - ( - None, - "", - Some(Mode::Dark), - style::DEFAULT_DARK_THEME, - Mode::Dark, - ), - ( - None, - "<@@@@@>", - Some(Mode::Light), - style::DEFAULT_LIGHT_THEME, - Mode::Light, - ), - (None, "1337", Some(Mode::Light), "1337", Mode::Light), - (Some("none".to_string()), "", None, "none", Mode::Dark), - ( - Some("None".to_string()), - "", - Some(Mode::Light), - "None", - Mode::Light, - ), - ] { - if bat_theme_env_var == "<not set>" { - env::remove_var("BAT_THEME") - } else { - env::set_var("BAT_THEME", bat_theme_env_var); - } - let is_true_color = true; - let mut options = get_command_line_options(); - options.theme = theme_option; - match mode_optio |