Line state refactor (#851)

* Refactor: compute raw line and new line state

6 files changed, 177 insertions, 149 deletions

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<LineSections<'a, SyntectStyle>>,
     diff_style_sections: Vec<LineSections<'a, Style>>,
     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<String> {
-        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<String> {
+    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<State> {
+fn new_line_state(
+    new_line: &str,
+    new_raw_line: &str,
+    prev_state: &State,
+    config: &Config,
+) -> Option<State> {
     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<State> {
         )),
     };
 
-    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<State> {
         _ => 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<Item = &'a str>,
-    {
-        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::<String>()
-        } else {
-            line.collect::<String>()
-        }
-    }
-
     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<Vec<LineSections<'a, Style>>>,
-        lines_have_homolog: &MinusPlus<Vec<bool>>,
-        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<LineSections<'a, Style>>,
+        diff_style_sections: &mut Vec<LineSections<'a, Style>>,
         whitespace_error_style: Option<Style>,
         non_emph_style: Option<Style>,
         lines_have_homolog: &[bool],
@@ -568,7 +499,7 @@ impl<'p> Painter<'p> {
     ) {
         for (((_, state), style_sections), line_has_homolog) in lines
             .iter()
-            .zip_eq(lines_style_sections)
+            .zip_eq(diff_style_sections)
             .zip_eq(lines_have_homolog)
         {
             if let State::HunkMinus(_, Some(raw_line))
@@ -606,6 +537,49 @@ 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(line: &str, prefix_length: usize, config: &config::Config) -> 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", expand_tabs(line, config.tab_width))
+    } else {
+        "\n".to_string()
+    }
+}
+
+// Remove initial -/+ characters, expand tabs as spaces, retaining ANSI sequences. Terminate with
+// newline character.
+pub fn prepare_raw_line(raw_line: &str, prefix_length: usize, config: &config::Config) -> String {
+    format!(
+        "{}\n",
+        ansi::ansi_preserving_slice(
+            &expand_tabs(raw_line.graphemes(true), config.tab_width),
+            prefix_length
+        ),
+    )
+}
+
+/// Expand tabs as spaces.
+/// tab_width = 0 is documented to mean do not replace tabs.
+pub fn expand_tabs<'a, I>(line: I, tab_width: usize) -> String
+where
+    I: Iterator<Item = &'a str>,
+{
+    if tab_width > 0 {
+        let tab_replacement = " ".repeat(tab_width);
+        line.map(|s| if s == "\t" { &tab_replacement } else { s })
+            .collect::<String>()
+    } else {
+        line.collect::<String>()
+    }
+}
+
 pub fn paint_minus_and_plus_lines(
     lines: MinusPlus<&Vec<(String, State)>>,
     line_numbers_data: &mut Option<LineNumbersData>,
@@ -620,9 +594,27 @@ pub fn paint_minus_and_plus_lines(
     let (mut diff_style_sections, line_alignment) = get_diff_style_sections(&lines, config);
     let lines_have_homolog = edits::make_lines_have_homolog(&line_alignment);
     Painter::update_diff_style_sections(
-        &lines,
-        &mut diff_style_sections,
-        &lines_have_homolog,
+        lines[Minus],
+        &mut diff_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,
+    );
+    Painter::update_diff_style_sections(
+        lines[Plus],
+        &mut diff_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,
     );
     if config.side_by_side {
diff --git a/src/wrapping.rs b/src/wrapping.rs
index e600ccc4..05fa4311 100644
--- a/src/wrapping.rs
+++ b/src/wrapping.rs
@@ -474,7 +474,7 @@ pub fn wrap_minusplus_block<'c: 'a, 'a>(
 #[allow(clippy::comparison_chain, clippy::type_complexity)]
 pub fn wrap_zero_block<'c: 'a, 'a>(
     config: &'c Config,
-    raw_line: &str,
+    line: &str,
     mut states: Vec<State>,
     syntax_style_sections: Vec<LineSections<'a, SyntectStyle>>,
     diff_style_sections: Vec<LineSections<'a, Style>>,
@@ -501,7 +501,7 @@ pub fn wrap_zero_block<'c: 'a, 'a>(
     // If that changes the wrapping logic should be updated as well.
     debug_assert_eq!(diff_style_sections.len(), 1);
 
-    let should_wrap = line_is_too_long(raw_line, line_width);
+    let should_wrap = line_is_too_long(line, line_width);
 
     if should_wrap {
         let syntax_style = wrap_line(