Line state refactor (#851)

* Refactor: compute raw line and new line state
This commit is contained in:
Dan Davison 2021-12-14 22:21:07 -05:00 committed by GitHub
parent d72ddfc6ad
commit b5d7c23e07
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 177 additions and 149 deletions

View File

@ -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,

View File

@ -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,

View File

@ -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,
}
}

View File

@ -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 {

View File

@ -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 {

View File

@ -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(