Skip to content

Commit

Permalink
Refactor: compute raw line together with new line state
Browse files Browse the repository at this point in the history
Refactor: introduce Line enum over line | raw_line
  • Loading branch information
dandavison committed Dec 14, 2021
1 parent 32d3d5a commit d90d09c
Show file tree
Hide file tree
Showing 8 changed files with 142 additions and 93 deletions.
13 changes: 10 additions & 3 deletions src/delta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>), // In hunk; unchanged line (prefix, raw_line)
HunkMinus(DiffType, Option<String>), // In hunk; removed line (diff_type, raw_line)
HunkPlus(DiffType, Option<String>), // 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
Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions src/features/side_by_side.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -234,7 +234,7 @@ pub fn paint_zero_lines_side_by_side<'a>(
painted_prefix: Option<ansi_term::ANSIString>,
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,
Expand Down
146 changes: 88 additions & 58 deletions src/handlers/hunk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -83,41 +83,27 @@ 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.
self.painter.paint_buffered_minus_and_plus_lines();
}
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.
Expand All @@ -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
Expand All @@ -149,42 +128,59 @@ 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()?;
Ok(true)
}
}

// 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
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<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,
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
Expand Down Expand Up @@ -220,7 +216,7 @@ fn new_line_state(new_line: &str, prev_state: &State) -> Option<State> {
};

// 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())];
Expand All @@ -240,21 +236,55 @@ fn new_line_state(new_line: &str, prev_state: &State) -> Option<State> {
_ => 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,
}
}
Expand Down
22 changes: 17 additions & 5 deletions src/handlers/merge_conflict.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -52,22 +52,31 @@ 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) => {
handled_line = self.enter_theirs(&merge_parents)
|| 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,
),
);
}
_ => {}
Expand Down Expand Up @@ -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(())
}
}
Expand Down
Loading

0 comments on commit d90d09c

Please sign in to comment.