Skip to content

Commit

Permalink
Gracefully handle failure to parse hunk header
Browse files Browse the repository at this point in the history
Fixes #765
  • Loading branch information
dandavison committed Dec 5, 2021
1 parent 9dc486e commit d042798
Show file tree
Hide file tree
Showing 8 changed files with 123 additions and 75 deletions.
2 changes: 1 addition & 1 deletion src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl Config {
State::HunkPlus(_, _) => &self.plus_style,
State::CommitMeta => &self.commit_style,
State::DiffHeader(_) => &self.file_style,
State::HunkHeader(_, _, _) => &self.hunk_header_style,
State::HunkHeader(_, _, _, _) => &self.hunk_header_style,
State::SubmoduleLog => &self.file_style,
_ => delta_unreachable("Unreachable code reached in get_style."),
}
Expand Down
3 changes: 2 additions & 1 deletion src/delta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::ansi;
use crate::config::delta_unreachable;
use crate::config::Config;
use crate::features;
use crate::handlers::hunk_header::ParsedHunkHeader;
use crate::handlers::{self, merge_conflict};
use crate::paint::Painter;
use crate::style::DecorationStyle;
Expand All @@ -17,7 +18,7 @@ use crate::style::DecorationStyle;
pub enum State {
CommitMeta, // In commit metadata section
DiffHeader(DiffType), // In diff metadata section, between (possible) commit metadata and first hunk
HunkHeader(DiffType, String, String), // In hunk metadata line (diff_type, line, raw_line)
HunkHeader(DiffType, ParsedHunkHeader, String, String), // In hunk metadata line (diff_type, parsed, line, raw_line)
HunkZero(DiffType), // In hunk; unchanged line (prefix)
HunkMinus(DiffType, Option<String>), // In hunk; removed line (diff_type, raw_line)
HunkPlus(DiffType, Option<String>), // In hunk; added line (diff_type, raw_line)
Expand Down
12 changes: 6 additions & 6 deletions src/handlers/hunk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl<'a> StateMachine<'a> {
fn test_hunk_line(&self) -> bool {
matches!(
self.state,
State::HunkHeader(_, _, _)
State::HunkHeader(_, _, _, _)
| State::HunkZero(_)
| State::HunkMinus(_, _)
| State::HunkPlus(_, _)
Expand Down Expand Up @@ -59,8 +59,8 @@ impl<'a> StateMachine<'a> {
{
self.painter.paint_buffered_minus_and_plus_lines();
}
if let State::HunkHeader(_, line, raw_line) = &self.state.clone() {
self.emit_hunk_header_line(line, raw_line)?;
if let State::HunkHeader(_, parsed_hunk_header, line, raw_line) = &self.state.clone() {
self.emit_hunk_header_line(parsed_hunk_header, line, raw_line)?;
}
self.state = match new_line_state(&self.line, &self.state) {
Some(HunkMinus(diff_type, _)) => {
Expand Down Expand Up @@ -140,13 +140,13 @@ fn new_line_state(new_line: &str, prev_state: &State) -> Option<State> {
HunkMinus(Unified, _)
| HunkZero(Unified)
| HunkPlus(Unified, _)
| HunkHeader(Unified, _, _) => Unified,
HunkHeader(Combined(Number(n), InMergeConflict::No), _, _) => {
| HunkHeader(Unified, _, _, _) => Unified,
HunkHeader(Combined(Number(n), InMergeConflict::No), _, _, _) => {
Combined(Number(*n), InMergeConflict::No)
}
// The prefixes are specific to the previous line, but the number of merge parents remains
// equal to the prefix length.
HunkHeader(Combined(Prefix(prefix), InMergeConflict::No), _, _) => {
HunkHeader(Combined(Prefix(prefix), InMergeConflict::No), _, _, _) => {
Combined(Number(prefix.len()), InMergeConflict::No)
}
HunkMinus(Combined(Prefix(prefix), in_merge_conflict), _)
Expand Down
165 changes: 103 additions & 62 deletions src/handlers/hunk_header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ use crate::delta::{self, DiffType, InMergeConflict, MergeParents, State, StateMa
use crate::paint::{self, BgShouldFill, Painter, StyleSectionSpecifier};
use crate::style::DecorationStyle;

#[derive(Clone, Default, Debug, PartialEq)]
pub struct ParsedHunkHeader {
code_fragment: String,
line_numbers_and_hunk_lengths: Vec<(usize, usize)>,
}

impl<'a> StateMachine<'a> {
#[inline]
fn test_hunk_header_line(&self) -> bool {
Expand All @@ -45,39 +51,57 @@ impl<'a> StateMachine<'a> {
if !self.test_hunk_header_line() {
return Ok(false);
}
let diff_type = match &self.state {
DiffHeader(Combined(MergeParents::Unknown, InMergeConflict::No)) => {
// https://git-scm.com/docs/git-diff#_combined_diff_format
let n_parents = self.line.chars().take_while(|c| c == &'@').count() - 1;
Combined(MergeParents::Number(n_parents), InMergeConflict::No)
}
DiffHeader(diff_type)
| HunkMinus(diff_type, _)
| HunkZero(diff_type)
| HunkPlus(diff_type, _) => diff_type.clone(),
Unknown => Unified,
_ => delta_unreachable(&format!(
"Unexpected state in handle_hunk_header: {:?}",
self.state
)),
};
self.state = HunkHeader(diff_type, self.line.clone(), self.raw_line.clone());
Ok(true)
let mut handled_line = false;
if let Some(parsed_hunk_header) = parse_hunk_header(&self.line) {
let diff_type = match &self.state {
DiffHeader(Combined(MergeParents::Unknown, InMergeConflict::No)) => {
// https://git-scm.com/docs/git-diff#_combined_diff_format
let n_parents = self.line.chars().take_while(|c| c == &'@').count() - 1;
Combined(MergeParents::Number(n_parents), InMergeConflict::No)
}
DiffHeader(diff_type)
| HunkMinus(diff_type, _)
| HunkZero(diff_type)
| HunkPlus(diff_type, _) => diff_type.clone(),
Unknown => Unified,
_ => delta_unreachable(&format!(
"Unexpected state in handle_hunk_header: {:?}",
self.state
)),
};
self.state = HunkHeader(
diff_type,
parsed_hunk_header,
self.line.clone(),
self.raw_line.clone(),
);
handled_line = true;
}
Ok(handled_line)
}

/// Emit the hunk header, with any requested decoration.
pub fn emit_hunk_header_line(&mut self, line: &str, raw_line: &str) -> std::io::Result<bool> {
pub fn emit_hunk_header_line(
&mut self,
parsed_hunk_header: &ParsedHunkHeader,
line: &str,
raw_line: &str,
) -> std::io::Result<bool> {
self.painter.paint_buffered_minus_and_plus_lines();
self.painter.set_highlighter();
self.painter.emit()?;

let (code_fragment, line_numbers) = parse_hunk_header(line);
let ParsedHunkHeader {
code_fragment,
line_numbers_and_hunk_lengths,
} = parsed_hunk_header;

if self.config.line_numbers {
self.painter
.line_numbers_data
.as_mut()
.unwrap()
.initialize_hunk(&line_numbers, self.plus_file.to_string());
.initialize_hunk(line_numbers_and_hunk_lengths, self.plus_file.to_string());
}

if self.config.hunk_header_style.is_raw {
Expand All @@ -92,8 +116,8 @@ impl<'a> StateMachine<'a> {
}

write_hunk_header(
&code_fragment,
&line_numbers,
code_fragment,
line_numbers_and_hunk_lengths,
&mut self.painter,
line,
if self.plus_file == "/dev/null" {
Expand Down Expand Up @@ -132,25 +156,31 @@ lazy_static! {
/// Given input like
/// "@@ -74,15 +74,14 @@ pub fn delta("
/// Return " pub fn delta(" and a vector of (line_number, hunk_length) tuples.
fn parse_hunk_header(line: &str) -> (String, Vec<(usize, usize)>) {
let caps = HUNK_HEADER_REGEX.captures(line).unwrap();
let file_coordinates = &caps[1];
let line_numbers_and_hunk_lengths = HUNK_HEADER_FILE_COORDINATE_REGEX
.captures_iter(file_coordinates)
.map(|caps| {
(
caps[1].parse::<usize>().unwrap(),
caps.get(2)
.map(|m| m.as_str())
// Per the specs linked above, if the hunk length is absent then it is 1.
.unwrap_or("1")
.parse::<usize>()
.unwrap(),
)
fn parse_hunk_header(line: &str) -> Option<ParsedHunkHeader> {
if let Some(caps) = HUNK_HEADER_REGEX.captures(line) {
let file_coordinates = &caps[1];
let line_numbers_and_hunk_lengths = HUNK_HEADER_FILE_COORDINATE_REGEX
.captures_iter(file_coordinates)
.map(|caps| {
(
caps[1].parse::<usize>().unwrap(),
caps.get(2)
.map(|m| m.as_str())
// Per the specs linked above, if the hunk length is absent then it is 1.
.unwrap_or("1")
.parse::<usize>()
.unwrap(),
)
})
.collect();
let code_fragment = caps[2].to_string();
Some(ParsedHunkHeader {
code_fragment,
line_numbers_and_hunk_lengths,
})
.collect();
let code_fragment = &caps[2];
(code_fragment.to_string(), line_numbers_and_hunk_lengths)
} else {
None
}
}

fn write_hunk_header_raw(
Expand All @@ -177,7 +207,7 @@ fn write_hunk_header_raw(

pub fn write_hunk_header(
code_fragment: &str,
line_numbers: &[(usize, usize)],
line_numbers_and_hunk_lengths: &[(usize, usize)],
painter: &mut Painter,
line: &str,
plus_file: &str,
Expand All @@ -193,7 +223,7 @@ pub fn write_hunk_header(
"".to_string()
};

let plus_line_number = line_numbers[line_numbers.len() - 1].0;
let plus_line_number = line_numbers_and_hunk_lengths[line_numbers_and_hunk_lengths.len() - 1].0;
let file_with_line_number =
paint_file_path_with_line_number(Some(plus_line_number), plus_file, config);

Expand Down Expand Up @@ -272,7 +302,12 @@ fn write_to_output_buffer(
painter.syntax_highlight_and_paint_line(
&line,
StyleSectionSpecifier::Style(config.hunk_header_style),
delta::State::HunkHeader(DiffType::Unified, "".to_owned(), "".to_owned()),
delta::State::HunkHeader(
DiffType::Unified,
ParsedHunkHeader::default(),
"".to_owned(),
"".to_owned(),
),
BgShouldFill::No,
);
painter.output_buffer.pop(); // trim newline
Expand All @@ -287,49 +322,54 @@ pub mod tests {

#[test]
fn test_parse_hunk_header() {
let parsed = parse_hunk_header("@@ -74,15 +75,14 @@ pub fn delta(\n");
let code_fragment = parsed.0;
let line_numbers_and_hunk_lengths = parsed.1;
let ParsedHunkHeader {
code_fragment,
line_numbers_and_hunk_lengths,
} = parse_hunk_header("@@ -74,15 +75,14 @@ pub fn delta(\n").unwrap();
assert_eq!(code_fragment, " pub fn delta(\n");
assert_eq!(line_numbers_and_hunk_lengths[0], (74, 15),);
assert_eq!(line_numbers_and_hunk_lengths[1], (75, 14),);
}

#[test]
fn test_parse_hunk_header_with_omitted_hunk_lengths() {
let parsed = parse_hunk_header("@@ -74 +75,2 @@ pub fn delta(\n");
let code_fragment = parsed.0;
let line_numbers_and_hunk_lengths = parsed.1;
let ParsedHunkHeader {
code_fragment,
line_numbers_and_hunk_lengths,
} = parse_hunk_header("@@ -74 +75,2 @@ pub fn delta(\n").unwrap();
assert_eq!(code_fragment, " pub fn delta(\n");
assert_eq!(line_numbers_and_hunk_lengths[0], (74, 1),);
assert_eq!(line_numbers_and_hunk_lengths[1], (75, 2),);
}

#[test]
fn test_parse_hunk_header_added_file() {
let parsed = parse_hunk_header("@@ -1,22 +0,0 @@");
let code_fragment = parsed.0;
let line_numbers_and_hunk_lengths = parsed.1;
let ParsedHunkHeader {
code_fragment,
line_numbers_and_hunk_lengths,
} = parse_hunk_header("@@ -1,22 +0,0 @@").unwrap();
assert_eq!(code_fragment, "",);
assert_eq!(line_numbers_and_hunk_lengths[0], (1, 22),);
assert_eq!(line_numbers_and_hunk_lengths[1], (0, 0),);
}

#[test]
fn test_parse_hunk_header_deleted_file() {
let parsed = parse_hunk_header("@@ -0,0 +1,3 @@");
let code_fragment = parsed.0;
let line_numbers_and_hunk_lengths = parsed.1;
let ParsedHunkHeader {
code_fragment,
line_numbers_and_hunk_lengths,
} = parse_hunk_header("@@ -0,0 +1,3 @@").unwrap();
assert_eq!(code_fragment, "",);
assert_eq!(line_numbers_and_hunk_lengths[0], (0, 0),);
assert_eq!(line_numbers_and_hunk_lengths[1], (1, 3),);
}

#[test]
fn test_parse_hunk_header_merge() {
let parsed = parse_hunk_header("@@@ -293,11 -358,15 +358,16 @@@ dependencies =");
let code_fragment = parsed.0;
let line_numbers_and_hunk_lengths = parsed.1;
let ParsedHunkHeader {
code_fragment,
line_numbers_and_hunk_lengths,
} = parse_hunk_header("@@@ -293,11 -358,15 +358,16 @@@ dependencies =").unwrap();
assert_eq!(code_fragment, " dependencies =");
assert_eq!(line_numbers_and_hunk_lengths[0], (293, 11),);
assert_eq!(line_numbers_and_hunk_lengths[1], (358, 15),);
Expand All @@ -338,9 +378,10 @@ pub mod tests {

#[test]
fn test_parse_hunk_header_cthulhu() {
let parsed = parse_hunk_header("@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -444,17 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 +444,17 @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ int snd_soc_jack_add_gpios(struct snd_s");
let code_fragment = parsed.0;
let line_numbers_and_hunk_lengths = parsed.1;
let ParsedHunkHeader {
code_fragment,
line_numbers_and_hunk_lengths,
} = parse_hunk_header("@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -444,17 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 -446,6 +444,17 @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ int snd_soc_jack_add_gpios(struct snd_s").unwrap();
assert_eq!(code_fragment, " int snd_soc_jack_add_gpios(struct snd_s");
assert_eq!(line_numbers_and_hunk_lengths[0], (446, 6),);
assert_eq!(line_numbers_and_hunk_lengths[1], (446, 6),);
Expand Down
2 changes: 1 addition & 1 deletion src/handlers/merge_conflict.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl<'a> StateMachine<'a> {
}

match self.state.clone() {
HunkHeader(Combined(merge_parents, InMergeConflict::No), _, _)
HunkHeader(Combined(merge_parents, InMergeConflict::No), _, _, _)
| HunkMinus(Combined(merge_parents, InMergeConflict::No), _)
| HunkZero(Combined(merge_parents, InMergeConflict::No))
| HunkPlus(Combined(merge_parents, InMergeConflict::No), _) => {
Expand Down
4 changes: 2 additions & 2 deletions src/handlers/submodule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ impl<'a> StateMachine<'a> {

#[inline]
fn test_submodule_short_line(&self) -> bool {
matches!(self.state, State::HunkHeader(_, _, _))
matches!(self.state, State::HunkHeader(_, _, _, _))
&& self.line.starts_with("-Subproject commit ")
|| matches!(self.state, State::SubmoduleShort(_))
&& self.line.starts_with("+Subproject commit ")
Expand All @@ -29,7 +29,7 @@ impl<'a> StateMachine<'a> {
return Ok(false);
}
if let Some(commit) = get_submodule_short_commit(&self.line) {
if let State::HunkHeader(_, _, _) = self.state {
if let State::HunkHeader(_, _, _, _) = self.state {
self.state = State::SubmoduleShort(commit.to_owned());
} else if let State::SubmoduleShort(minus_commit) = &self.state {
self.painter.emit()?;
Expand Down
2 changes: 1 addition & 1 deletion src/paint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ impl<'p> Painter<'p> {
|| config.plus_emph_style.is_syntax_highlighted
|| config.plus_non_emph_style.is_syntax_highlighted
}
State::HunkHeader(_, _, _) => true,
State::HunkHeader(_, _, _, _) => true,
State::HunkMinus(_, Some(_raw_line)) | State::HunkPlus(_, Some(_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
Expand Down
8 changes: 7 additions & 1 deletion src/tests/test_example_diffs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ mod tests {
use crate::ansi::{self, strip_ansi_codes};
use crate::cli::InspectRawLines;
use crate::delta::{DiffType, State};
use crate::handlers::hunk_header::ParsedHunkHeader;
use crate::style;
use crate::tests::ansi_test_utils::ansi_test_utils;
use crate::tests::integration_test_utils;
Expand Down Expand Up @@ -1382,7 +1383,12 @@ src/align.rs:71: impl<'a> Alignment<'a> { │
4,
"impl<'a> Alignment<'a> { ",
"rs",
State::HunkHeader(DiffType::Unified, "".to_owned(), "".to_owned()),
State::HunkHeader(
DiffType::Unified,
ParsedHunkHeader::default(),
"".to_owned(),
"".to_owned(),
),
&config,
);
ansi_test_utils::assert_line_has_no_color(&output, 12, "─────────────────────────────┘");
Expand Down

0 comments on commit d042798

Please sign in to comment.