From b5d7c23e07aa1ce97edc9a017c6d6abda7e758ce Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Tue, 14 Dec 2021 22:21:07 -0500 Subject: Line state refactor (#851) * Refactor: compute raw line and new line state --- src/features/side_by_side.rs | 4 +- src/handlers/grep.rs | 17 +++-- src/handlers/hunk.rs | 147 ++++++++++++++++++++++++---------------- src/handlers/merge_conflict.rs | 4 +- src/paint.rs | 150 +++++++++++++++++++---------------------- src/wrapping.rs | 4 +- 6 files changed, 177 insertions(+), 149 deletions(-) (limited to 'src') diff --git a/src/features/side_by_side.rs b/src/features/side_by_side.rs index 5b0dd5e9..96431696 100644 --- a/src/features/side_by_side.rs +++ b/src/features/side_by_side.rs @@ -225,7 +225,7 @@ pub fn paint_minus_and_plus_lines_side_by_side( #[allow(clippy::too_many_arguments)] pub fn paint_zero_lines_side_by_side<'a>( - raw_line: &str, + line: &str, syntax_style_sections: Vec>, diff_style_sections: Vec>, output_buffer: &mut String, @@ -238,7 +238,7 @@ pub fn paint_zero_lines_side_by_side<'a>( let (states, syntax_style_sections, diff_style_sections) = wrap_zero_block( config, - raw_line, + line, states, syntax_style_sections, diff_style_sections, diff --git a/src/handlers/grep.rs b/src/handlers/grep.rs index 4a76bf62..32e1463c 100644 --- a/src/handlers/grep.rs +++ b/src/handlers/grep.rs @@ -8,7 +8,7 @@ use unicode_segmentation::UnicodeSegmentation; use crate::ansi; use crate::delta::{State, StateMachine}; use crate::handlers::{self, ripgrep_json}; -use crate::paint::{self, BgShouldFill, StyleSectionSpecifier}; +use crate::paint::{self, expand_tabs, BgShouldFill, StyleSectionSpecifier}; use crate::style::Style; use crate::utils::process; @@ -139,10 +139,11 @@ impl<'a> StateMachine<'a> { // (At the time of writing, we are in this // arm iff we are handling `ripgrep --json` // output.) - grep_line.code = self - .painter - .expand_tabs(grep_line.code.graphemes(true)) - .into(); + grep_line.code = paint::expand_tabs( + grep_line.code.graphemes(true), + self.config.tab_width, + ) + .into(); make_style_sections( &grep_line.code, submatches, @@ -157,8 +158,10 @@ impl<'a> StateMachine<'a> { // enough. But at the point it is guaranteed // that this handler is going to handle this // line, so mutating it is acceptable. - self.raw_line = - self.painter.expand_tabs(self.raw_line.graphemes(true)); + self.raw_line = expand_tabs( + self.raw_line.graphemes(true), + self.config.tab_width, + ); get_code_style_sections( &self.raw_line, self.config.grep_match_word_style, diff --git a/src/handlers/hunk.rs b/src/handlers/hunk.rs index 460dd6c8..2acf2eae 100644 --- a/src/handlers/hunk.rs +++ b/src/handlers/hunk.rs @@ -3,8 +3,9 @@ use std::cmp::min; use lazy_static::lazy_static; use crate::cli; -use crate::config::delta_unreachable; +use crate::config::{delta_unreachable, Config}; use crate::delta::{DiffType, InMergeConflict, MergeParents, State, StateMachine}; +use crate::paint::{expand_tabs, prepare, prepare_raw_line}; use crate::style; use crate::utils::process::{self, CallingProcess}; use unicode_segmentation::UnicodeSegmentation; @@ -79,40 +80,27 @@ impl<'a> StateMachine<'a> { if let State::HunkHeader(_, parsed_hunk_header, line, raw_line) = &self.state.clone() { self.emit_hunk_header_line(parsed_hunk_header, line, raw_line)?; } - // TODO: The following code is pretty convoluted currently and has been volving. It may be - // heading for (state, line | raw_line) to be held together in an enum variant representing - // a line. - self.state = match new_line_state(&self.line, &self.state) { - Some(HunkMinus(diff_type, _)) => { + self.state = match new_line_state(&self.line, &self.raw_line, &self.state, self.config) { + Some(HunkMinus(diff_type, raw_line)) => { if let HunkPlus(_, _) = self.state { // We have just entered a new subhunk; process the previous one // and flush the line buffers. self.painter.paint_buffered_minus_and_plus_lines(); } let n_parents = diff_type.n_parents(); - let line = self.painter.prepare(&self.line, n_parents); - let raw_line = self.maybe_raw_line( - HunkMinus(diff_type.clone(), None), // TODO - n_parents, - &[*style::GIT_DEFAULT_MINUS_STYLE, self.config.git_minus_style], - ); + let line = prepare(&self.line, n_parents, self.config); let state = HunkMinus(diff_type, raw_line); self.painter.minus_lines.push((line, state.clone())); state } - Some(HunkPlus(diff_type, _)) => { + Some(HunkPlus(diff_type, raw_line)) => { let n_parents = diff_type.n_parents(); - let line = self.painter.prepare(&self.line, n_parents); - let raw_line = self.maybe_raw_line( - HunkPlus(diff_type.clone(), None), // TODO - n_parents, - &[*style::GIT_DEFAULT_PLUS_STYLE, self.config.git_plus_style], - ); + let line = prepare(&self.line, n_parents, self.config); let state = HunkPlus(diff_type, raw_line); self.painter.plus_lines.push((line, state.clone())); state } - Some(HunkZero(diff_type, _)) => { + Some(HunkZero(diff_type, raw_line)) => { // We are in a zero (unchanged) line, therefore we have just exited a subhunk (a // sequence of consecutive minus (removed) and/or plus (added) lines). Process that // subhunk and flush the line buffers. @@ -122,9 +110,7 @@ impl<'a> StateMachine<'a> { } else { diff_type.n_parents() }; - let line = self.painter.prepare(&self.line, n_parents); - let raw_line = - self.maybe_raw_line(HunkZero(diff_type.clone(), None), n_parents, &[]); // TODO + let line = prepare(&self.line, n_parents, self.config); let state = State::HunkZero(diff_type, raw_line); self.painter.paint_zero_line(&line, state.clone()); state @@ -134,9 +120,10 @@ impl<'a> StateMachine<'a> { // is not a hunk line, but the parser does not have a more accurate state corresponding // to this. self.painter.paint_buffered_minus_and_plus_lines(); - self.painter - .output_buffer - .push_str(&self.painter.expand_tabs(self.raw_line.graphemes(true))); + self.painter.output_buffer.push_str(&expand_tabs( + self.raw_line.graphemes(true), + self.config.tab_width, + )); self.painter.output_buffer.push('\n'); State::HunkZero(Unified, None) } @@ -144,37 +131,49 @@ impl<'a> StateMachine<'a> { self.painter.emit()?; Ok(true) } +} - // Return Some(prepared_raw_line) if delta should emit this line raw. - fn maybe_raw_line( - &self, - state: State, - n_parents: usize, - non_raw_styles: &[style::Style], - ) -> Option { - let emit_raw_line = is_word_diff() - || self.config.inspect_raw_lines == cli::InspectRawLines::True - && style::line_has_style_other_than(&self.raw_line, non_raw_styles) - || self.config.get_style(&state).is_raw; - if emit_raw_line { - Some(self.painter.prepare_raw_line(&self.raw_line, n_parents)) - } else { - None - } +// Return Some(prepared_raw_line) if delta should emit this line raw. +fn maybe_raw_line( + raw_line: &str, + state_style_is_raw: bool, + n_parents: usize, + non_raw_styles: &[style::Style], + config: &Config, +) -> Option { + let emit_raw_line = is_word_diff() + || config.inspect_raw_lines == cli::InspectRawLines::True + && style::line_has_style_other_than(raw_line, non_raw_styles) + || state_style_is_raw; + if emit_raw_line { + Some(prepare_raw_line(raw_line, n_parents, config)) + } else { + None } } // Return the new state corresponding to `new_line`, given the previous state. A return value of // None means that `new_line` is not recognized as a hunk line. -fn new_line_state(new_line: &str, prev_state: &State) -> Option { +fn new_line_state( + new_line: &str, + new_raw_line: &str, + prev_state: &State, + config: &Config, +) -> Option { use DiffType::*; use MergeParents::*; use State::*; if is_word_diff() { - return Some(HunkZero(Unified, None)); + return Some(HunkZero( + Unified, + maybe_raw_line(new_raw_line, config.zero_style.is_raw, 0, &[], config), + )); } + // 1. Given the previous line state, compute the new line diff type. These are basically the + // same, except that a string prefix is converted into an integer number of parents (prefix + // length). let diff_type = match prev_state { HunkMinus(Unified, _) | HunkZero(Unified, _) @@ -204,7 +203,8 @@ fn new_line_state(new_line: &str, prev_state: &State) -> Option { )), }; - let (prefix_char, prefix, in_merge_conflict) = match diff_type { + // 2. Given the new diff state, and the new line, compute the new prefix. + let (prefix_char, prefix, in_merge_conflict) = match diff_type.clone() { Unified => (new_line.chars().next(), None, None), Combined(Number(n_parents), in_merge_conflict) => { let prefix = &new_line[..min(n_parents, new_line.len())]; @@ -224,19 +224,52 @@ fn new_line_state(new_line: &str, prev_state: &State) -> Option { _ => delta_unreachable(""), }; + let maybe_minus_raw_line = || { + maybe_raw_line( + new_raw_line, + config.minus_style.is_raw, + diff_type.n_parents(), + &[*style::GIT_DEFAULT_MINUS_STYLE, config.git_minus_style], + config, + ) + }; + let maybe_zero_raw_line = || { + maybe_raw_line( + new_raw_line, + config.zero_style.is_raw, + diff_type.n_parents(), + &[], + config, + ) + }; + let maybe_plus_raw_line = || { + maybe_raw_line( + new_raw_line, + config.plus_style.is_raw, + diff_type.n_parents(), + &[*style::GIT_DEFAULT_PLUS_STYLE, config.git_plus_style], + config, + ) + }; + + // 3. Given the new prefix, compute the full new line state...except without its raw_line, which + // is added later. TODO: that is not a sensible design. match (prefix_char, prefix, in_merge_conflict) { - (Some('-'), None, None) => Some(HunkMinus(Unified, None)), - (Some(' '), None, None) => Some(HunkZero(Unified, None)), - (Some('+'), None, None) => Some(HunkPlus(Unified, None)), - (Some('-'), Some(prefix), Some(in_merge_conflict)) => { - Some(HunkMinus(Combined(Prefix(prefix), in_merge_conflict), None)) - } - (Some(' '), Some(prefix), Some(in_merge_conflict)) => { - Some(HunkZero(Combined(Prefix(prefix), in_merge_conflict), None)) - } - (Some('+'), Some(prefix), Some(in_merge_conflict)) => { - Some(HunkPlus(Combined(Prefix(prefix), in_merge_conflict), None)) - } + (Some('-'), None, None) => Some(HunkMinus(Unified, maybe_minus_raw_line())), + (Some(' '), None, None) => Some(HunkZero(Unified, maybe_zero_raw_line())), + (Some('+'), None, None) => Some(HunkPlus(Unified, maybe_plus_raw_line())), + (Some('-'), Some(prefix), Some(in_merge_conflict)) => Some(HunkMinus( + Combined(Prefix(prefix), in_merge_conflict), + maybe_minus_raw_line(), + )), + (Some(' '), Some(prefix), Some(in_merge_conflict)) => Some(HunkZero( + Combined(Prefix(prefix), in_merge_conflict), + maybe_zero_raw_line(), + )), + (Some('+'), Some(prefix), Some(in_merge_conflict)) => Some(HunkPlus( + Combined(Prefix(prefix), in_merge_conflict), + maybe_plus_raw_line(), + )), _ => None, } } diff --git a/src/handlers/merge_conflict.rs b/src/handlers/merge_conflict.rs index 26f9b199..926d7a3c 100644 --- a/src/handlers/merge_conflict.rs +++ b/src/handlers/merge_conflict.rs @@ -8,7 +8,7 @@ use crate::cli; use crate::config::{self, delta_unreachable}; use crate::delta::{DiffType, InMergeConflict, MergeParents, State, StateMachine}; use crate::minusplus::MinusPlus; -use crate::paint; +use crate::paint::{self, prepare}; use crate::style::Style; #[derive(Clone, Debug, PartialEq)] @@ -121,7 +121,7 @@ impl<'a> StateMachine<'a> { fn store_line(&mut self, commit: MergeConflictCommit, state: State) -> bool { use State::*; if let HunkMinus(diff_type, _) | HunkZero(diff_type, _) | HunkPlus(diff_type, _) = &state { - let line = self.painter.prepare(&self.line, diff_type.n_parents()); + let line = prepare(&self.line, diff_type.n_parents(), self.config); self.painter.merge_conflict_lines[commit].push((line, state)); true } else { diff --git a/src/paint.rs b/src/paint.rs index 0852c9fa..32c724da 100644 --- a/src/paint.rs +++ b/src/paint.rs @@ -124,46 +124,6 @@ impl<'p> Painter<'p> { }; } - /// Remove initial -/+ character, expand tabs as spaces, and terminate with newline. - // Terminating with newline character is necessary for many of the sublime syntax definitions to - // highlight correctly. - // See https://docs.rs/syntect/3.2.0/syntect/parsing/struct.SyntaxSetBuilder.html#method.add_from_folder - pub fn prepare(&self, line: &str, prefix_length: usize) -> String { - if !line.is_empty() { - // The prefix contains -/+/space characters, added by git. We removes them now so they - // are not present during syntax highlighting or wrapping. If --keep-plus-minus-markers - // is in effect the prefix is re-inserted in Painter::paint_line. - let line = line.graphemes(true).skip(prefix_length); - format!("{}\n", self.expand_tabs(line)) - } else { - "\n".to_string() - } - } - - // Remove initial -/+ characters, expand tabs as spaces, retaining ANSI sequences. Terminate with - // newline character. - pub fn prepare_raw_line(&self, raw_line: &str, prefix_length: usize) -> String { - format!( - "{}\n", - ansi::ansi_preserving_slice(&self.expand_tabs(raw_line.graphemes(true)), prefix_length), - ) - } - - /// Expand tabs as spaces. - /// tab_width = 0 is documented to mean do not replace tabs. - pub fn expand_tabs<'a, I>(&self, line: I) -> String - where - I: Iterator, - { - if self.config.tab_width > 0 { - let tab_replacement = " ".repeat(self.config.tab_width); - line.map(|s| if s == "\t" { &tab_replacement } else { s }) - .collect::() - } else { - line.collect::() - } - } - pub fn paint_buffered_minus_and_plus_lines(&mut self) { paint_minus_and_plus_lines( MinusPlus::new(&self.minus_lines, &self.plus_lines), @@ -181,7 +141,7 @@ impl<'p> Painter<'p> { let syntax_style_sections = get_syntax_style_sections_for_lines(lines, self.highlighter.as_mut(), self.config); let mut diff_style_sections = vec![vec![(self.config.zero_style, lines[0].0.as_str())]]; // TODO: compute style from state - Painter::update_styles( + Painter::update_diff_style_sections( lines, &mut diff_style_sections, None, @@ -296,7 +256,10 @@ impl<'p> Painter<'p> { state: State, background_color_extends_to_terminal_width: BgShouldFill, ) { - let lines = vec![(self.expand_tabs(line.graphemes(true)), state)]; + let lines = vec![( + expand_tabs(line.graphemes(true), self.config.tab_width), + state, + )]; let syntax_style_sections = get_syntax_style_sections_for_lines(&lines, self.highlighter.as_mut(), self.config); let diff_style_sections = match style_sections { @@ -527,40 +490,8 @@ impl<'p> Painter<'p> { /// computed diff styles with these styles from the raw line. (This is /// how support for git's --color-moved is implemented.) fn update_diff_style_sections<'a>( - lines: &MinusPlus<&'a Vec<(String, State)>>, - lines_style_sections: &mut MinusPlus>>, - lines_have_homolog: &MinusPlus>, - config: &config::Config, - ) { - Self::update_styles( - lines[Minus], - &mut lines_style_sections[Minus], - None, - if config.minus_non_emph_style != config.minus_emph_style { - Some(config.minus_non_emph_style) - } else { - None - }, - &lines_have_homolog[Minus], - config, - ); - Self::update_styles( - lines[Plus], - &mut lines_style_sections[Plus], - Some(config.whitespace_error_style), - if config.plus_non_emph_style != config.plus_emph_style { - Some(config.plus_non_emph_style) - } else { - None - }, - &lines_have_homolog[Plus], - config, - ); - } - - fn update_styles<'a>( lines: &'a [(String, State)], - lines_style_sections: &mut Vec>, + diff_style_sections: &mut Vec>, whitespace_error_style: Option