From d90d09c2d3280c44f1ba470923f11f1502fd0676 Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Sun, 12 Dec 2021 23:16:32 -0500 Subject: [PATCH] Refactor: compute raw line together with new line state Refactor: introduce Line enum over line | raw_line --- src/delta.rs | 13 ++- src/features/side_by_side.rs | 8 +- src/handlers/hunk.rs | 146 +++++++++++++++++++------------- src/handlers/merge_conflict.rs | 22 +++-- src/paint.rs | 32 +++---- src/subcommands/show_colors.rs | 4 +- src/tests/test_example_diffs.rs | 4 +- src/wrapping.rs | 6 +- 8 files changed, 142 insertions(+), 93 deletions(-) diff --git a/src/delta.rs b/src/delta.rs index 7fbcdf483..829aae2a9 100644 --- a/src/delta.rs +++ b/src/delta.rs @@ -19,9 +19,9 @@ pub enum State { CommitMeta, // In commit metadata section DiffHeader(DiffType), // In diff metadata section, between (possible) commit metadata and first hunk HunkHeader(DiffType, ParsedHunkHeader, String, String), // In hunk metadata line (diff_type, parsed, line, raw_line) - HunkZero(DiffType, Option), // In hunk; unchanged line (prefix, raw_line) - HunkMinus(DiffType, Option), // In hunk; removed line (diff_type, raw_line) - HunkPlus(DiffType, Option), // In hunk; added line (diff_type, raw_line) + HunkZero(DiffType, Line), // In hunk; unchanged line + HunkMinus(DiffType, Line), // In hunk; removed line + HunkPlus(DiffType, Line), // In hunk; added line MergeConflict(MergeParents, merge_conflict::MergeConflictCommit), SubmoduleLog, // In a submodule section, with gitconfig diff.submodule = log SubmoduleShort(String), // In a submodule section, with gitconfig diff.submodule = short @@ -42,6 +42,13 @@ pub enum DiffType { Combined(MergeParents, InMergeConflict), } +#[derive(Debug, Clone, PartialEq)] +pub enum Line { + // Line(String), + RawLine(String), + Placeholder, +} + #[derive(Clone, Debug, PartialEq)] pub enum MergeParents { Number(usize), // Number of parent commits == (number of @s in hunk header) - 1 diff --git a/src/features/side_by_side.rs b/src/features/side_by_side.rs index 44f17a679..97ae169b1 100644 --- a/src/features/side_by_side.rs +++ b/src/features/side_by_side.rs @@ -6,7 +6,7 @@ use crate::ansi; use crate::cli; use crate::config::{self, delta_unreachable, Config}; use crate::delta::DiffType; -use crate::delta::State; +use crate::delta::{Line, State}; use crate::edits; use crate::features::{line_numbers, OptionValueFunction}; use crate::minusplus::*; @@ -176,7 +176,7 @@ pub fn paint_minus_and_plus_lines_side_by_side( for (minus_line_index, plus_line_index) in line_alignment { let left_state = match minus_line_index { Some(i) => &line_states[Left][i], - None => &State::HunkMinus(DiffType::Unified, None), + None => &State::HunkMinus(DiffType::Unified, Line::Placeholder), }; output_buffer.push_str(&paint_left_panel_minus_line( minus_line_index, @@ -191,7 +191,7 @@ pub fn paint_minus_and_plus_lines_side_by_side( let right_state = match plus_line_index { Some(i) => &line_states[Right][i], - None => &State::HunkPlus(DiffType::Unified, None), + None => &State::HunkPlus(DiffType::Unified, Line::Placeholder), }; output_buffer.push_str(&paint_right_panel_plus_line( plus_line_index, @@ -234,7 +234,7 @@ pub fn paint_zero_lines_side_by_side<'a>( painted_prefix: Option, background_color_extends_to_terminal_width: BgShouldFill, ) { - let states = vec![State::HunkZero(DiffType::Unified, None)]; + let states = vec![State::HunkZero(DiffType::Unified, Line::Placeholder)]; let (states, syntax_style_sections, diff_style_sections) = wrap_zero_block( config, diff --git a/src/handlers/hunk.rs b/src/handlers/hunk.rs index 415ecf070..57a112035 100644 --- a/src/handlers/hunk.rs +++ b/src/handlers/hunk.rs @@ -4,7 +4,7 @@ use lazy_static::lazy_static; use crate::cli; use crate::config::{delta_unreachable, Config}; -use crate::delta::{DiffType, InMergeConflict, MergeParents, State, StateMachine}; +use crate::delta::{DiffType, InMergeConflict, Line, MergeParents, State, StateMachine}; use crate::paint::{expand_tabs, prepare, prepare_raw_line}; use crate::style; use crate::utils::process::{self, CallingProcess}; @@ -83,8 +83,8 @@ impl<'a> StateMachine<'a> { // TODO: The following code is pretty convoluted currently and has been evolving. 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. @@ -92,32 +92,18 @@ impl<'a> StateMachine<'a> { } let n_parents = diff_type.n_parents(); let line = prepare(&self.line, n_parents, self.config); - let raw_line = maybe_raw_line( - &self.raw_line, - self.config.minus_style.is_raw, - n_parents, - &[*style::GIT_DEFAULT_MINUS_STYLE, self.config.git_minus_style], - 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 = prepare(&self.line, n_parents, self.config); - let raw_line = maybe_raw_line( - &self.raw_line, - self.config.plus_style.is_raw, - n_parents, - &[*style::GIT_DEFAULT_PLUS_STYLE, self.config.git_plus_style], - 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. @@ -128,13 +114,6 @@ impl<'a> StateMachine<'a> { diff_type.n_parents() }; let line = prepare(&self.line, n_parents, self.config); - let raw_line = maybe_raw_line( - &self.raw_line, - self.config.zero_style.is_raw, - n_parents, - &[], - self.config, - ); // TODO let state = State::HunkZero(diff_type, raw_line); self.painter.paint_zero_line(&line, state.clone()); state @@ -149,7 +128,7 @@ impl<'a> StateMachine<'a> { self.config.tab_width, )); self.painter.output_buffer.push('\n'); - State::HunkZero(Unified, None) + State::HunkZero(Unified, Line::Placeholder) } }; self.painter.emit()?; @@ -157,34 +136,51 @@ impl<'a> StateMachine<'a> { } } -// 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 +impl Line { + fn new( + _line: &str, + raw_line: &str, + state_style_is_raw: bool, + n_parents: usize, + non_raw_styles: &[style::Style], + config: &Config, + ) -> Self { + 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 { + Line::RawLine(prepare_raw_line(raw_line, n_parents, config)) + } else { + Line::Placeholder + } } } // 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, + Line::new( + new_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 @@ -220,7 +216,7 @@ fn new_line_state(new_line: &str, prev_state: &State) -> Option { }; // 2. Given the new diff state, and the new line, compute the new prefix. - let (prefix_char, prefix, in_merge_conflict) = match diff_type { + 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())]; @@ -240,21 +236,55 @@ fn new_line_state(new_line: &str, prev_state: &State) -> Option { _ => delta_unreachable(""), }; + let make_minus_line = || { + Line::new( + new_line, + new_raw_line, + config.minus_style.is_raw, + diff_type.n_parents(), + &[*style::GIT_DEFAULT_MINUS_STYLE, config.git_minus_style], + config, + ) + }; + let make_zero_line = || { + Line::new( + new_line, + new_raw_line, + config.zero_style.is_raw, + diff_type.n_parents(), + &[], + config, + ) + }; + let make_plus_line = || { + Line::new( + new_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, make_minus_line())), + (Some(' '), None, None) => Some(HunkZero(Unified, make_zero_line())), + (Some('+'), None, None) => Some(HunkPlus(Unified, make_plus_line())), + (Some('-'), Some(prefix), Some(in_merge_conflict)) => Some(HunkMinus( + Combined(Prefix(prefix), in_merge_conflict), + make_minus_line(), + )), + (Some(' '), Some(prefix), Some(in_merge_conflict)) => Some(HunkZero( + Combined(Prefix(prefix), in_merge_conflict), + make_zero_line(), + )), + (Some('+'), Some(prefix), Some(in_merge_conflict)) => Some(HunkPlus( + Combined(Prefix(prefix), in_merge_conflict), + make_plus_line(), + )), _ => None, } } diff --git a/src/handlers/merge_conflict.rs b/src/handlers/merge_conflict.rs index 926d7a3c5..75474132b 100644 --- a/src/handlers/merge_conflict.rs +++ b/src/handlers/merge_conflict.rs @@ -6,7 +6,7 @@ use unicode_segmentation::UnicodeSegmentation; use super::draw; use crate::cli; use crate::config::{self, delta_unreachable}; -use crate::delta::{DiffType, InMergeConflict, MergeParents, State, StateMachine}; +use crate::delta::{DiffType, InMergeConflict, Line, MergeParents, State, StateMachine}; use crate::minusplus::MinusPlus; use crate::paint::{self, prepare}; use crate::style::Style; @@ -52,7 +52,10 @@ impl<'a> StateMachine<'a> { || self.exit_merge_conflict(&merge_parents)? || self.store_line( Ours, - HunkPlus(Combined(merge_parents, InMergeConflict::Yes), None), + HunkPlus( + Combined(merge_parents, InMergeConflict::Yes), + Line::Placeholder, + ), ); } MergeConflict(merge_parents, Ancestral) => { @@ -60,14 +63,20 @@ impl<'a> StateMachine<'a> { || self.exit_merge_conflict(&merge_parents)? || self.store_line( Ancestral, - HunkMinus(Combined(merge_parents, InMergeConflict::Yes), None), + HunkMinus( + Combined(merge_parents, InMergeConflict::Yes), + Line::Placeholder, + ), ); } MergeConflict(merge_parents, Theirs) => { handled_line = self.exit_merge_conflict(&merge_parents)? || self.store_line( Theirs, - HunkPlus(Combined(merge_parents, InMergeConflict::Yes), None), + HunkPlus( + Combined(merge_parents, InMergeConflict::Yes), + Line::Placeholder, + ), ); } _ => {} @@ -172,7 +181,10 @@ impl<'a> StateMachine<'a> { self.config, )?; self.painter.merge_conflict_lines.clear(); - self.state = HunkZero(Combined(merge_parents.clone(), InMergeConflict::No), None); + self.state = HunkZero( + Combined(merge_parents.clone(), InMergeConflict::No), + Line::Placeholder, + ); Ok(()) } } diff --git a/src/paint.rs b/src/paint.rs index 6a1556876..934d86c7e 100644 --- a/src/paint.rs +++ b/src/paint.rs @@ -9,7 +9,7 @@ use syntect::parsing::{SyntaxReference, SyntaxSet}; use unicode_segmentation::UnicodeSegmentation; use crate::config::{self, delta_unreachable, Config}; -use crate::delta::{DiffType, InMergeConflict, MergeParents, State}; +use crate::delta::{DiffType, InMergeConflict, Line, MergeParents, State}; use crate::edits; use crate::features::hyperlinks; use crate::features::line_numbers::{self, LineNumbersData}; @@ -289,24 +289,24 @@ impl<'p> Painter<'p> { config: &config::Config, ) -> (Option, Style) { let fill_style = match state { - State::HunkMinus(_, None) | State::HunkMinusWrapped => { + State::HunkMinus(_, Line::Placeholder) | State::HunkMinusWrapped => { if let Some(true) = line_has_homolog { config.minus_non_emph_style } else { config.minus_style } } - State::HunkZero(_, None) | State::HunkZeroWrapped => config.zero_style, - State::HunkPlus(_, None) | State::HunkPlusWrapped => { + State::HunkZero(_, Line::Placeholder) | State::HunkZeroWrapped => config.zero_style, + State::HunkPlus(_, Line::Placeholder) | State::HunkPlusWrapped => { if let Some(true) = line_has_homolog { config.plus_non_emph_style } else { config.plus_style } } - State::HunkMinus(_, Some(_)) - | State::HunkZero(_, Some(_)) - | State::HunkPlus(_, Some(_)) => { + State::HunkMinus(_, Line::RawLine(_)) + | State::HunkZero(_, Line::RawLine(_)) + | State::HunkPlus(_, Line::RawLine(_)) => { // Consider the following raw line, from git colorMoved: // ␛[1;36m+␛[m␛[1;36mclass·X:·pass␛[m␊ The last style section returned by // parse_style_sections will be a default style associated with the terminal newline @@ -437,21 +437,21 @@ impl<'p> Painter<'p> { return false; } match state { - State::HunkMinus(_, None) => { + State::HunkMinus(_, Line::Placeholder) => { config.minus_style.is_syntax_highlighted || config.minus_emph_style.is_syntax_highlighted || config.minus_non_emph_style.is_syntax_highlighted } - State::HunkZero(_, None) => config.zero_style.is_syntax_highlighted, - State::HunkPlus(_, None) => { + State::HunkZero(_, Line::Placeholder) => config.zero_style.is_syntax_highlighted, + State::HunkPlus(_, Line::Placeholder) => { config.plus_style.is_syntax_highlighted || config.plus_emph_style.is_syntax_highlighted || config.plus_non_emph_style.is_syntax_highlighted } State::HunkHeader(_, _, _, _) => true, - State::HunkMinus(_, Some(_raw_line)) - | State::HunkZero(_, Some(_raw_line)) - | State::HunkPlus(_, Some(_raw_line)) => { + State::HunkMinus(_, Line::RawLine(_raw_line)) + | State::HunkZero(_, Line::RawLine(_raw_line)) + | State::HunkPlus(_, Line::RawLine(_raw_line)) => { // It is possible that the captured raw line contains an ANSI // style that has been mapped (via map-styles) to a delta Style // with syntax-highlighting. @@ -534,9 +534,9 @@ impl<'p> Painter<'p> { .zip_eq(lines_style_sections) .zip_eq(lines_have_homolog) { - if let State::HunkMinus(_, Some(raw_line)) - | State::HunkZero(_, Some(raw_line)) - | State::HunkPlus(_, Some(raw_line)) = state + if let State::HunkMinus(_, Line::RawLine(raw_line)) + | State::HunkZero(_, Line::RawLine(raw_line)) + | State::HunkPlus(_, Line::RawLine(raw_line)) = state { // raw_line is captured in handle_hunk_line under certain conditions. If we have // done so, then overwrite the style sections with styles parsed directly from the diff --git a/src/subcommands/show_colors.rs b/src/subcommands/show_colors.rs index 1c939700f..a8da67acf 100644 --- a/src/subcommands/show_colors.rs +++ b/src/subcommands/show_colors.rs @@ -45,7 +45,7 @@ pub fn show_colors() -> std::io::Result<()> { painter.syntax_highlight_and_paint_line( line, paint::StyleSectionSpecifier::Style(style), - delta::State::HunkZero(DiffType::Unified, None), + delta::State::HunkZero(DiffType::Unified, delta::Line::Placeholder), BgShouldFill::default(), ) } @@ -64,7 +64,7 @@ pub fn show_colors() -> std::io::Result<()> { painter.syntax_highlight_and_paint_line( line, paint::StyleSectionSpecifier::Style(style), - delta::State::HunkZero(DiffType::Unified, None), + delta::State::HunkZero(DiffType::Unified, delta::Line::Placeholder), BgShouldFill::default(), ) } diff --git a/src/tests/test_example_diffs.rs b/src/tests/test_example_diffs.rs index ce917c362..531f74527 100644 --- a/src/tests/test_example_diffs.rs +++ b/src/tests/test_example_diffs.rs @@ -2,7 +2,7 @@ mod tests { use crate::ansi::{self, strip_ansi_codes}; use crate::cli::InspectRawLines; - use crate::delta::{DiffType, State}; + use crate::delta::{DiffType, Line, State}; use crate::handlers::hunk_header::ParsedHunkHeader; use crate::style; use crate::tests::ansi_test_utils::ansi_test_utils; @@ -1543,7 +1543,7 @@ src/align.rs:71: impl<'a> Alignment<'a> { │ 1, " for (i, x_i) in self.x.iter().enumerate() {", "rs", - State::HunkZero(DiffType::Unified, None), + State::HunkZero(DiffType::Unified, Line::Placeholder), &config, ); } diff --git a/src/wrapping.rs b/src/wrapping.rs index e600ccc4d..cdf24f3e3 100644 --- a/src/wrapping.rs +++ b/src/wrapping.rs @@ -5,7 +5,7 @@ use crate::config::INLINE_SYMBOL_WIDTH_1; use crate::config::Config; use crate::delta::DiffType; -use crate::delta::State; +use crate::delta::{Line, State}; use crate::features::line_numbers::{self, SideBySideLineWidth}; use crate::features::side_by_side::{available_line_width, line_is_too_long, Left, Right}; use crate::minusplus::*; @@ -450,13 +450,13 @@ pub fn wrap_minusplus_block<'c: 'a, 'a>( }; if minus_extended > 0 { - new_states[Left].push(State::HunkMinus(DiffType::Unified, None)); + new_states[Left].push(State::HunkMinus(DiffType::Unified, Line::Placeholder)); for _ in 1..minus_extended { new_states[Left].push(State::HunkMinusWrapped); } } if plus_extended > 0 { - new_states[Right].push(State::HunkPlus(DiffType::Unified, None)); + new_states[Right].push(State::HunkPlus(DiffType::Unified, Line::Placeholder)); for _ in 1..plus_extended { new_states[Right].push(State::HunkPlusWrapped); }