From 19d2459e3a59f6bc46e6a25361bcf58f09594f5e Mon Sep 17 00:00:00 2001 From: Roman Miklashevskii Date: Wed, 14 May 2025 14:50:21 +0300 Subject: [PATCH 01/13] Parse 'no new line at end' indicator as part of line. Fixes https://github.com/gitpatch-rs/gitpatch/issues/4 --- examples/apply.rs | 13 ++++-- src/ast.rs | 88 ++++++++++++++++++++++++++++++++-------- src/parser.rs | 95 +++++++++++++++++++------------------------- tests/parse_patch.rs | 52 ++++++++++++++++++++---- tests/regressions.rs | 6 +-- 5 files changed, 168 insertions(+), 86 deletions(-) diff --git a/examples/apply.rs b/examples/apply.rs index 150365e..464fd1f 100644 --- a/examples/apply.rs +++ b/examples/apply.rs @@ -6,6 +6,7 @@ fn apply(diff: Patch, old: &str) -> String { let old_lines = old.lines().collect::>(); let mut out: Vec<&str> = vec![]; let mut old_line = 0; + let mut need_new_line = false; for hunk in diff.hunks { while old_line < hunk.old_range.start - 1 { out.push(old_lines[old_line as usize]); @@ -14,12 +15,18 @@ fn apply(diff: Patch, old: &str) -> String { old_line += hunk.old_range.count; for line in hunk.lines { match line { - Line::Add(s) | Line::Context(s) => out.push(s), - Line::Remove(_) => {} + Line::Add(s, end_newline) | Line::Context(s, end_newline) => { + out.push(s); + need_new_line = end_newline; + } + Line::Remove(_, _) => {} } } } - out.join("\n") + if need_new_line { + out.push(""); + } + out.join("") } static LAO: &str = "\ diff --git a/src/ast.rs b/src/ast.rs index d923f10..a9b0224 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -14,11 +14,33 @@ pub struct Patch<'a> { pub new: File<'a>, /// hunks of differences; each hunk shows one area where the files differ pub hunks: Vec>, - /// true if the last line of the file ends in a newline character - /// - /// This will only be false if at the end of the patch we encounter the text: - /// `\ No newline at end of file` - pub end_newline: bool, +} + +impl<'a> Patch<'a> { + pub fn end_newline_before(&self) -> bool { + self.end_newline(Line::is_remove_or_context) + } + + pub fn end_newline_after(&self) -> bool { + self.end_newline(Line::is_add_or_context) + } + + fn end_newline(&self, mut line_filter: impl FnMut(&Line<'a>) -> bool) -> bool { + let Some(last_hunk) = self.hunks.last() else { + return true; + }; + + let Some(last_line) = last_hunk + .lines + .iter() + .filter(|&line| line_filter(line)) + .next_back() + else { + return true; + }; + + last_line.end_newline() + } } impl fmt::Display for Patch<'_> { @@ -31,9 +53,6 @@ impl fmt::Display for Patch<'_> { for hunk in &self.hunks { write!(f, "\n{}", hunk)?; } - if !self.end_newline { - write!(f, "\n\\ No newline at end of file")?; - } Ok(()) } } @@ -72,7 +91,7 @@ impl<'a> Patch<'a> { /// let patch = Patch::from_single(sample)?; /// assert_eq!(&patch.old.path, "lao"); /// assert_eq!(&patch.new.path, "tzu"); - /// assert_eq!(patch.end_newline, false); + /// assert_eq!(patch.end_newline_before() && patch.end_newline_after(), false); /// # Ok(()) /// # } /// ``` @@ -269,23 +288,58 @@ impl fmt::Display for Range { #[derive(Debug, Clone, Eq, PartialEq)] pub enum Line<'a> { /// A line added to the old file in the new file - Add(&'a str), + Add(&'a str, bool), /// A line removed from the old file in the new file - Remove(&'a str), + Remove(&'a str, bool), /// A line provided for context in the diff (unchanged); from both the old and the new file - Context(&'a str), + Context(&'a str, bool), } -impl fmt::Display for Line<'_> { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { +impl<'a> Line<'a> { + pub fn add(line: &'a str) -> Self { + Line::Add(line, true) + } + pub fn remove(line: &'a str) -> Self { + Line::Remove(line, true) + } + pub fn context(line: &'a str) -> Self { + Line::Context(line, true) + } + + pub fn is_add_or_context(&self) -> bool { + matches!(self, Line::Add(..) | Line::Context(..)) + } + + pub fn is_remove_or_context(&self) -> bool { + matches!(self, Line::Remove(..) | Line::Context(..)) + } + + pub fn end_newline(&self) -> bool { match self { - Line::Add(line) => write!(f, "+{}", line), - Line::Remove(line) => write!(f, "-{}", line), - Line::Context(line) => write!(f, " {}", line), + Line::Add(_, end_newline) => *end_newline, + Line::Remove(_, end_newline) => *end_newline, + Line::Context(_, end_newline) => *end_newline, } } } +impl fmt::Display for Line<'_> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let (prefix, &line, &end_newline) = match self { + Line::Add(line, end_newline) => ("+", line, end_newline), + Line::Remove(line, end_newline) => ("-", line, end_newline), + Line::Context(line, end_newline) => (" ", line, end_newline), + }; + + write!(f, "{prefix}{line}")?; + if !end_newline { + write!(f, "\n\\ No newline at end of file")?; + }; + + Ok(()) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/parser.rs b/src/parser.rs index 8c7b00b..f4e9a21 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -63,9 +63,15 @@ impl Error for ParseError<'_> { } } -fn consume_content_line(input: Input<'_>) -> IResult, &str> { +fn consume_content_line(input: Input<'_>) -> IResult, (&str, bool)> { let (input, raw) = terminated(not_line_ending, line_ending)(input)?; - Ok((input, raw.fragment())) + + let (input, no_newline) = opt(terminated( + tag("\\ No newline at end of file"), + opt(line_ending), + ))(input)?; + + Ok((input, (raw.fragment(), no_newline.is_none()))) } pub(crate) fn parse_single_patch(s: &str) -> Result> { @@ -107,20 +113,11 @@ fn patch(input: Input<'_>) -> IResult, Patch> { } let (input, files) = headers(input)?; let (input, hunks) = chunks(input)?; - let (input, no_newline_indicator) = no_newline_indicator(input)?; // Ignore trailing empty lines produced by some diff programs let (input, _) = many0(line_ending)(input)?; let (old, new) = files; - Ok(( - input, - Patch { - old, - new, - hunks, - end_newline: !no_newline_indicator, - }, - )) + Ok((input, Patch { old, new, hunks })) } /// Recognize a "binary files XX and YY differ" line as an empty patch. @@ -152,7 +149,6 @@ fn binary_files_differ(input: Input<'_>) -> IResult, Patch> { meta: None, }, hunks: Vec::new(), - end_newline: false, }, )) } @@ -182,7 +178,6 @@ fn file_rename_only(input: Input<'_>) -> IResult, Patch> { meta: None, }, hunks: Vec::new(), - end_newline: false, }, )) } @@ -235,7 +230,6 @@ fn is_next_header(input: Input<'_>) -> bool { || input.starts_with("@@ ") } - /// Looks for lines starting with + or - or space, but not +++ or ---. Not a foolproof check. /// /// For example, if someone deletes a line that was using the pre-decrement (--) operator or adds a @@ -273,17 +267,20 @@ fn chunk(input: Input<'_>) -> IResult, Hunk> { // Detect added lines map( preceded(tuple((char('+'), not(tag("++ ")))), consume_content_line), - Line::Add, + |(line, end_newline)| Line::Add(line, end_newline), ), // Detect removed lines map( preceded(tuple((char('-'), not(tag("-- ")))), consume_content_line), - Line::Remove, + |(line, end_newline)| Line::Remove(line, end_newline), ), // Detect context lines - map(preceded(char(' '), consume_content_line), Line::Context), + map( + preceded(char(' '), consume_content_line), + |(line, end_newline)| Line::Context(line, end_newline), + ), // Handle empty lines within the chunk - map(tag("\n"), |_| Line::Context("")), + map(tag("\n"), |_| Line::Context("", false)), )), // Stop parsing when we detect the next header or have parsed the expected number of lines |_| !is_next_header(input), @@ -327,17 +324,6 @@ fn u64_digit(input: Input<'_>) -> IResult, u64> { Ok((input, num)) } -// Trailing newline indicator -fn no_newline_indicator(input: Input<'_>) -> IResult, bool> { - map( - opt(terminated( - tag("\\ No newline at end of file"), - opt(line_ending), - )), - |matched| matched.is_some(), - )(input) -} - fn filename(input: Input<'_>) -> IResult, Cow> { alt((quoted, bare))(input) } @@ -584,15 +570,15 @@ mod tests { new_range: Range { start: 1, count: 6 }, range_hint: "", lines: vec![ - Line::Remove("The Way that can be told of is not the eternal Way;"), - Line::Remove("The name that can be named is not the eternal name."), - Line::Context("The Nameless is the origin of Heaven and Earth;"), - Line::Remove("The Named is the mother of all things."), - Line::Add("The named is the mother of all things."), - Line::Add(""), - Line::Context("Therefore let there always be non-being,"), - Line::Context(" so we may see their subtlety,"), - Line::Context("And let there always be being,"), + Line::remove("The Way that can be told of is not the eternal Way;"), + Line::remove("The name that can be named is not the eternal name."), + Line::context("The Nameless is the origin of Heaven and Earth;"), + Line::remove("The Named is the mother of all things."), + Line::add("The named is the mother of all things."), + Line::add(""), + Line::context("Therefore let there always be non-being,"), + Line::context(" so we may see their subtlety,"), + Line::context("And let there always be being,"), ], }; test_parser!(chunk(sample) -> expected); @@ -642,15 +628,15 @@ mod tests { new_range: Range { start: 1, count: 6 }, range_hint: "", lines: vec![ - Line::Remove("The Way that can be told of is not the eternal Way;"), - Line::Remove("The name that can be named is not the eternal name."), - Line::Context("The Nameless is the origin of Heaven and Earth;"), - Line::Remove("The Named is the mother of all things."), - Line::Add("The named is the mother of all things."), - Line::Add(""), - Line::Context("Therefore let there always be non-being,"), - Line::Context(" so we may see their subtlety,"), - Line::Context("And let there always be being,"), + Line::remove("The Way that can be told of is not the eternal Way;"), + Line::remove("The name that can be named is not the eternal name."), + Line::context("The Nameless is the origin of Heaven and Earth;"), + Line::remove("The Named is the mother of all things."), + Line::add("The named is the mother of all things."), + Line::add(""), + Line::context("Therefore let there always be non-being,"), + Line::context(" so we may see their subtlety,"), + Line::context("And let there always be being,"), ], }, Hunk { @@ -658,16 +644,15 @@ mod tests { new_range: Range { start: 8, count: 6 }, range_hint: "", lines: vec![ - Line::Context("The two are the same,"), - Line::Context("But after they are produced,"), - Line::Context(" they have different names."), - Line::Add("They both may be called deep and profound."), - Line::Add("Deeper and more profound,"), - Line::Add("The door of all subtleties!"), + Line::context("The two are the same,"), + Line::context("But after they are produced,"), + Line::context(" they have different names."), + Line::add("They both may be called deep and profound."), + Line::add("Deeper and more profound,"), + Line::add("The door of all subtleties!"), ], }, ], - end_newline: true, }; test_parser!(patch(sample) -> expected); diff --git a/tests/parse_patch.rs b/tests/parse_patch.rs index da25ada..2c6c382 100644 --- a/tests/parse_patch.rs +++ b/tests/parse_patch.rs @@ -31,7 +31,7 @@ fn test_parse() -> Result<(), ParseError<'static>> { meta: None } ); - assert!(patch.end_newline); + assert!(patch.end_newline_after() && patch.end_newline_after()); assert_eq!(format!("{}\n", patch), sample); @@ -67,7 +67,43 @@ fn test_parse_no_newline_indicator() -> Result<(), ParseError<'static>> { meta: None } ); - assert!(!patch.end_newline); + assert!(!patch.end_newline_before() && !patch.end_newline_after()); + + assert_eq!(format!("{}\n", patch), sample); + + Ok(()) +} + +#[test] +fn test_parse_no_newline_indicator_inside() -> Result<(), ParseError<'static>> { + let sample = "\ +--- test.txt ++++ test.txt +@@ -1,4 +1,5 @@ + python + eggy + hamster +-guido +\\ No newline at end of file ++ ++hello +\\ No newline at end of file\n"; + let patch = Patch::from_single(sample)?; + assert_eq!( + patch.old, + File { + path: "test.txt".into(), + meta: None + } + ); + assert_eq!( + patch.new, + File { + path: "test.txt".into(), + meta: None + } + ); + assert!(!patch.end_newline_before() && !patch.end_newline_after()); assert_eq!(format!("{}\n", patch), sample); @@ -107,7 +143,7 @@ fn test_parse_timestamps() -> Result<(), ParseError<'static>> { )), } ); - assert!(!patch.end_newline); + assert!(!patch.end_newline_after() && !patch.end_newline_before()); // to_string() uses Display but adds no trailing newline assert_eq!(patch.to_string(), sample); @@ -147,7 +183,7 @@ fn test_parse_other() -> Result<(), ParseError<'static>> { )), } ); - assert!(patch.end_newline); + assert!(patch.end_newline_before() && patch.end_newline_after()); assert_eq!(format!("{}\n", patch), sample); @@ -184,7 +220,7 @@ fn test_parse_escaped() -> Result<(), ParseError<'static>> { )), } ); - assert!(patch.end_newline); + assert!(patch.end_newline_before() && patch.end_newline_after()); assert_eq!(format!("{}\n", patch), sample); @@ -226,7 +262,7 @@ fn test_parse_triple_plus_minus() -> Result<(), ParseError<'static>> { meta: None } ); - assert!(patch.end_newline); + assert!(patch.end_newline_before() && patch.end_newline_after()); assert_eq!(patch.hunks.len(), 1); assert_eq!(patch.hunks[0].lines.len(), 8); @@ -278,7 +314,7 @@ fn test_parse_triple_plus_minus_hack() { meta: None } ); - assert!(patch.end_newline); + assert!(patch.end_newline_before() && patch.end_newline_after()); assert_eq!(patch.hunks.len(), 1); assert_eq!(patch.hunks[0].lines.len(), 8); @@ -397,7 +433,7 @@ index d923f10..b47f160 100644 assert!(patches[0].hunks[0] .lines .iter() - .any(|line| matches!(line, Line::Add("use new_crate;")))); + .any(|line| matches!(line, Line::Add("use new_crate;", true)))); } #[test] diff --git a/tests/regressions.rs b/tests/regressions.rs index 04cd59f..03c9657 100644 --- a/tests/regressions.rs +++ b/tests/regressions.rs @@ -11,7 +11,7 @@ fn hunk_header_context_is_not_a_line_15() -> Result<(), ParseError<'static>> { x "; let patch = Patch::from_single(sample)?; - assert_eq!(patch.hunks[0].lines, [Line::Context("x")]); + assert_eq!(patch.hunks[0].lines, [Line::context("x")]); Ok(()) } @@ -39,11 +39,11 @@ fn crlf_breaks_stuff_17() -> Result<(), ParseError<'static>> { old_range: Range { start: 0, count: 0 }, new_range: Range { start: 0, count: 0 }, range_hint: "", - lines: vec![Line::Context("x")], + lines: vec![Line::context("x")], }], - end_newline: true, } ); + assert!(patch.end_newline_before() && patch.end_newline_after()); Ok(()) } From b1b5e8f4178d896ffb950191e65d041652449f48 Mon Sep 17 00:00:00 2001 From: Roman Miklashevskii Date: Wed, 14 May 2025 18:17:08 +0300 Subject: [PATCH 02/13] refactor Line to struct, calculate missing_newline during parsing, remove functions to calculate them --- examples/apply.rs | 12 ++--- src/ast.rs | 108 ++++++++++++++++++------------------------ src/parser.rs | 109 +++++++++++++++++++++++++++++++------------ tests/parse_patch.rs | 30 ++++++------ tests/regressions.rs | 11 +++-- 5 files changed, 154 insertions(+), 116 deletions(-) diff --git a/examples/apply.rs b/examples/apply.rs index 464fd1f..01b0e67 100644 --- a/examples/apply.rs +++ b/examples/apply.rs @@ -1,6 +1,6 @@ //! Demonstrates how to apply a parsed diff to a file -use gitpatch::{Line, Patch}; +use gitpatch::{LineKind, Patch}; fn apply(diff: Patch, old: &str) -> String { let old_lines = old.lines().collect::>(); @@ -14,12 +14,12 @@ fn apply(diff: Patch, old: &str) -> String { } old_line += hunk.old_range.count; for line in hunk.lines { - match line { - Line::Add(s, end_newline) | Line::Context(s, end_newline) => { - out.push(s); - need_new_line = end_newline; + match line.kind { + LineKind::Add | LineKind::Context => { + out.push(line.content); + need_new_line = !line.missing_newline; } - Line::Remove(_, _) => {} + LineKind::Remove => {} } } } diff --git a/src/ast.rs b/src/ast.rs index a9b0224..0129049 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -12,37 +12,14 @@ pub struct Patch<'a> { pub old: File<'a>, /// The file information of the `+` side of the diff, line prefix: `+++` pub new: File<'a>, + /// If there was a `No newline at end of file` indicator after the last line of the old version of the file + pub old_missing_newline: bool, + /// If there was a `No newline at end of file` indicator after the last line of the new version of the file + pub new_missing_newline: bool, /// hunks of differences; each hunk shows one area where the files differ pub hunks: Vec>, } -impl<'a> Patch<'a> { - pub fn end_newline_before(&self) -> bool { - self.end_newline(Line::is_remove_or_context) - } - - pub fn end_newline_after(&self) -> bool { - self.end_newline(Line::is_add_or_context) - } - - fn end_newline(&self, mut line_filter: impl FnMut(&Line<'a>) -> bool) -> bool { - let Some(last_hunk) = self.hunks.last() else { - return true; - }; - - let Some(last_line) = last_hunk - .lines - .iter() - .filter(|&line| line_filter(line)) - .next_back() - else { - return true; - }; - - last_line.end_newline() - } -} - impl fmt::Display for Patch<'_> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { // Display implementations typically hold up the invariant that there is no trailing @@ -91,7 +68,7 @@ impl<'a> Patch<'a> { /// let patch = Patch::from_single(sample)?; /// assert_eq!(&patch.old.path, "lao"); /// assert_eq!(&patch.new.path, "tzu"); - /// assert_eq!(patch.end_newline_before() && patch.end_newline_after(), false); + /// assert_eq!(patch.new_missing_newline && patch.old_missing_newline, false); /// # Ok(()) /// # } /// ``` @@ -237,6 +214,10 @@ pub struct Hunk<'a> { pub new_range: Range, /// Any trailing text after the hunk's range information pub range_hint: &'a str, + /// If there was a `No newline at end of file` indicator after the last line of the old version of the hunk + pub old_missing_newline: bool, + /// If there was a `No newline at end of file` indicator after the last line of the new version of the hunk + pub new_missing_newline: bool, /// Each line of text in the hunk, prefixed with the type of change it represents pub lines: Vec>, } @@ -284,55 +265,56 @@ impl fmt::Display for Range { } } -/// A line of the old file, new file, or both -#[derive(Debug, Clone, Eq, PartialEq)] -pub enum Line<'a> { +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum LineKind { /// A line added to the old file in the new file - Add(&'a str, bool), + Add, /// A line removed from the old file in the new file - Remove(&'a str, bool), + Remove, /// A line provided for context in the diff (unchanged); from both the old and the new file - Context(&'a str, bool), + Context, } -impl<'a> Line<'a> { - pub fn add(line: &'a str) -> Self { - Line::Add(line, true) - } - pub fn remove(line: &'a str) -> Self { - Line::Remove(line, true) - } - pub fn context(line: &'a str) -> Self { - Line::Context(line, true) - } - - pub fn is_add_or_context(&self) -> bool { - matches!(self, Line::Add(..) | Line::Context(..)) - } - - pub fn is_remove_or_context(&self) -> bool { - matches!(self, Line::Remove(..) | Line::Context(..)) +impl LineKind { + pub fn to_line_full(self, line: &str, missing_newline: bool) -> Line<'_> { + Line { + kind: self, + content: line, + missing_newline, + } } - pub fn end_newline(&self) -> bool { - match self { - Line::Add(_, end_newline) => *end_newline, - Line::Remove(_, end_newline) => *end_newline, - Line::Context(_, end_newline) => *end_newline, + pub fn to_line(self, line: &str) -> Line<'_> { + Line { + kind: self, + content: line, + missing_newline: false, } } } +/// A line of the old file, new file, or both +#[derive(Debug, Clone, Eq, PartialEq)] +pub struct Line<'a> { + /// The kind of the line (added, removed, or context) + pub kind: LineKind, + /// The actual text of the line + pub content: &'a str, + /// If there was a `No newline at end of file` indicator after the line + pub missing_newline: bool, +} + impl fmt::Display for Line<'_> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let (prefix, &line, &end_newline) = match self { - Line::Add(line, end_newline) => ("+", line, end_newline), - Line::Remove(line, end_newline) => ("-", line, end_newline), - Line::Context(line, end_newline) => (" ", line, end_newline), + let prefix = match self.kind { + LineKind::Add => "+", + LineKind::Remove => "-", + LineKind::Context => " ", }; - write!(f, "{prefix}{line}")?; - if !end_newline { + write!(f, "{}{}", prefix, self.content)?; + + if self.missing_newline { write!(f, "\n\\ No newline at end of file")?; }; @@ -351,6 +333,8 @@ mod tests { old_range: Range { start: 0, count: 0 }, new_range: Range { start: 0, count: 0 }, range_hint: "", + old_missing_newline: false, + new_missing_newline: false, lines: vec![], }; for (input, expected) in vec![ diff --git a/src/parser.rs b/src/parser.rs index f4e9a21..d2ef720 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -66,12 +66,12 @@ impl Error for ParseError<'_> { fn consume_content_line(input: Input<'_>) -> IResult, (&str, bool)> { let (input, raw) = terminated(not_line_ending, line_ending)(input)?; - let (input, no_newline) = opt(terminated( + let (input, missing_newline) = opt(terminated( tag("\\ No newline at end of file"), opt(line_ending), ))(input)?; - Ok((input, (raw.fragment(), no_newline.is_none()))) + Ok((input, (raw.fragment(), missing_newline.is_some()))) } pub(crate) fn parse_single_patch(s: &str) -> Result> { @@ -116,8 +116,22 @@ fn patch(input: Input<'_>) -> IResult, Patch> { // Ignore trailing empty lines produced by some diff programs let (input, _) = many0(line_ending)(input)?; + let (old_missing_newline, new_missing_newline) = hunks + .last() + .map(|hunk| (hunk.old_missing_newline, hunk.new_missing_newline)) + .unwrap_or((false, false)); + let (old, new) = files; - Ok((input, Patch { old, new, hunks })) + Ok(( + input, + Patch { + old, + new, + hunks, + old_missing_newline, + new_missing_newline, + }, + )) } /// Recognize a "binary files XX and YY differ" line as an empty patch. @@ -149,6 +163,8 @@ fn binary_files_differ(input: Input<'_>) -> IResult, Patch> { meta: None, }, hunks: Vec::new(), + old_missing_newline: false, + new_missing_newline: false, }, )) } @@ -178,6 +194,8 @@ fn file_rename_only(input: Input<'_>) -> IResult, Patch> { meta: None, }, hunks: Vec::new(), + old_missing_newline: false, + new_missing_newline: false, }, )) } @@ -267,25 +285,41 @@ fn chunk(input: Input<'_>) -> IResult, Hunk> { // Detect added lines map( preceded(tuple((char('+'), not(tag("++ ")))), consume_content_line), - |(line, end_newline)| Line::Add(line, end_newline), + |(line, missing_newline)| LineKind::Add.to_line_full(line, missing_newline), ), // Detect removed lines map( preceded(tuple((char('-'), not(tag("-- ")))), consume_content_line), - |(line, end_newline)| Line::Remove(line, end_newline), + |(line, missing_newline)| LineKind::Remove.to_line_full(line, missing_newline), ), // Detect context lines map( preceded(char(' '), consume_content_line), - |(line, end_newline)| Line::Context(line, end_newline), + |(line, missing_newline)| LineKind::Context.to_line_full(line, missing_newline), ), // Handle empty lines within the chunk - map(tag("\n"), |_| Line::Context("", false)), + map(tag("\n"), |_| LineKind::Context.to_line_full("", false)), )), // Stop parsing when we detect the next header or have parsed the expected number of lines |_| !is_next_header(input), ))(input)?; + // Track whether the last line for each side of the hunk is missing a final newline + let mut new_missing_newline = false; + let mut old_missing_newline = false; + for line in &lines { + let is_new = matches!(line.kind, LineKind::Add | LineKind::Context); + let is_old = matches!(line.kind, LineKind::Remove | LineKind::Context); + + if is_new { + new_missing_newline = line.missing_newline; + } + + if is_old { + old_missing_newline = line.missing_newline; + } + } + let (old_range, new_range, range_hint) = ranges; Ok(( input, @@ -294,6 +328,8 @@ fn chunk(input: Input<'_>) -> IResult, Hunk> { new_range, range_hint, lines, + old_missing_newline, + new_missing_newline, }, )) } @@ -569,16 +605,18 @@ mod tests { old_range: Range { start: 1, count: 7 }, new_range: Range { start: 1, count: 6 }, range_hint: "", + old_missing_newline: false, + new_missing_newline: false, lines: vec![ - Line::remove("The Way that can be told of is not the eternal Way;"), - Line::remove("The name that can be named is not the eternal name."), - Line::context("The Nameless is the origin of Heaven and Earth;"), - Line::remove("The Named is the mother of all things."), - Line::add("The named is the mother of all things."), - Line::add(""), - Line::context("Therefore let there always be non-being,"), - Line::context(" so we may see their subtlety,"), - Line::context("And let there always be being,"), + LineKind::Remove.to_line("The Way that can be told of is not the eternal Way;"), + LineKind::Remove.to_line("The name that can be named is not the eternal name."), + LineKind::Context.to_line("The Nameless is the origin of Heaven and Earth;"), + LineKind::Remove.to_line("The Named is the mother of all things."), + LineKind::Add.to_line("The named is the mother of all things."), + LineKind::Add.to_line(""), + LineKind::Context.to_line("Therefore let there always be non-being,"), + LineKind::Context.to_line(" so we may see their subtlety,"), + LineKind::Context.to_line("And let there always be being,"), ], }; test_parser!(chunk(sample) -> expected); @@ -622,34 +660,43 @@ mod tests { DateTime::parse_from_rfc3339("2002-02-21T23:30:50.442260588-08:00").unwrap(), )), }, + old_missing_newline: false, + new_missing_newline: false, hunks: vec![ Hunk { old_range: Range { start: 1, count: 7 }, new_range: Range { start: 1, count: 6 }, range_hint: "", + old_missing_newline: false, + new_missing_newline: false, lines: vec![ - Line::remove("The Way that can be told of is not the eternal Way;"), - Line::remove("The name that can be named is not the eternal name."), - Line::context("The Nameless is the origin of Heaven and Earth;"), - Line::remove("The Named is the mother of all things."), - Line::add("The named is the mother of all things."), - Line::add(""), - Line::context("Therefore let there always be non-being,"), - Line::context(" so we may see their subtlety,"), - Line::context("And let there always be being,"), + LineKind::Remove + .to_line("The Way that can be told of is not the eternal Way;"), + LineKind::Remove + .to_line("The name that can be named is not the eternal name."), + LineKind::Context + .to_line("The Nameless is the origin of Heaven and Earth;"), + LineKind::Remove.to_line("The Named is the mother of all things."), + LineKind::Add.to_line("The named is the mother of all things."), + LineKind::Add.to_line(""), + LineKind::Context.to_line("Therefore let there always be non-being,"), + LineKind::Context.to_line(" so we may see their subtlety,"), + LineKind::Context.to_line("And let there always be being,"), ], }, Hunk { old_range: Range { start: 9, count: 3 }, new_range: Range { start: 8, count: 6 }, range_hint: "", + old_missing_newline: false, + new_missing_newline: false, lines: vec![ - Line::context("The two are the same,"), - Line::context("But after they are produced,"), - Line::context(" they have different names."), - Line::add("They both may be called deep and profound."), - Line::add("Deeper and more profound,"), - Line::add("The door of all subtleties!"), + LineKind::Context.to_line("The two are the same,"), + LineKind::Context.to_line("But after they are produced,"), + LineKind::Context.to_line(" they have different names."), + LineKind::Add.to_line("They both may be called deep and profound."), + LineKind::Add.to_line("Deeper and more profound,"), + LineKind::Add.to_line("The door of all subtleties!"), ], }, ], diff --git a/tests/parse_patch.rs b/tests/parse_patch.rs index 2c6c382..882c616 100644 --- a/tests/parse_patch.rs +++ b/tests/parse_patch.rs @@ -1,5 +1,5 @@ use chrono::DateTime; -use gitpatch::{File, FileMetadata, Line, ParseError, Patch}; +use gitpatch::{File, FileMetadata, Line, LineKind, ParseError, Patch}; use pretty_assertions::assert_eq; @@ -31,7 +31,7 @@ fn test_parse() -> Result<(), ParseError<'static>> { meta: None } ); - assert!(patch.end_newline_after() && patch.end_newline_after()); + assert!(!patch.old_missing_newline && !patch.new_missing_newline); assert_eq!(format!("{}\n", patch), sample); @@ -67,7 +67,7 @@ fn test_parse_no_newline_indicator() -> Result<(), ParseError<'static>> { meta: None } ); - assert!(!patch.end_newline_before() && !patch.end_newline_after()); + assert!(patch.old_missing_newline && patch.new_missing_newline); assert_eq!(format!("{}\n", patch), sample); @@ -103,7 +103,7 @@ fn test_parse_no_newline_indicator_inside() -> Result<(), ParseError<'static>> { meta: None } ); - assert!(!patch.end_newline_before() && !patch.end_newline_after()); + assert!(patch.old_missing_newline && patch.new_missing_newline); assert_eq!(format!("{}\n", patch), sample); @@ -143,7 +143,7 @@ fn test_parse_timestamps() -> Result<(), ParseError<'static>> { )), } ); - assert!(!patch.end_newline_after() && !patch.end_newline_before()); + assert!(patch.old_missing_newline && patch.new_missing_newline); // to_string() uses Display but adds no trailing newline assert_eq!(patch.to_string(), sample); @@ -183,7 +183,7 @@ fn test_parse_other() -> Result<(), ParseError<'static>> { )), } ); - assert!(patch.end_newline_before() && patch.end_newline_after()); + assert!(!patch.old_missing_newline && !patch.new_missing_newline); assert_eq!(format!("{}\n", patch), sample); @@ -220,7 +220,7 @@ fn test_parse_escaped() -> Result<(), ParseError<'static>> { )), } ); - assert!(patch.end_newline_before() && patch.end_newline_after()); + assert!(!patch.old_missing_newline && !patch.new_missing_newline); assert_eq!(format!("{}\n", patch), sample); @@ -262,7 +262,7 @@ fn test_parse_triple_plus_minus() -> Result<(), ParseError<'static>> { meta: None } ); - assert!(patch.end_newline_before() && patch.end_newline_after()); + assert!(!patch.old_missing_newline && !patch.new_missing_newline); assert_eq!(patch.hunks.len(), 1); assert_eq!(patch.hunks[0].lines.len(), 8); @@ -314,7 +314,7 @@ fn test_parse_triple_plus_minus_hack() { meta: None } ); - assert!(patch.end_newline_before() && patch.end_newline_after()); + assert!(!patch.old_missing_newline && !patch.new_missing_newline); assert_eq!(patch.hunks.len(), 1); assert_eq!(patch.hunks[0].lines.len(), 8); @@ -430,10 +430,14 @@ index d923f10..b47f160 100644 assert_eq!(patches[0].old.path, "a/src/ast.rs"); assert_eq!(patches[0].new.path, "b/src/ast-2.rs"); - assert!(patches[0].hunks[0] - .lines - .iter() - .any(|line| matches!(line, Line::Add("use new_crate;", true)))); + assert!(patches[0].hunks[0].lines.iter().any(|line| matches!( + line, + Line { + kind: LineKind::Add, + content: "use new_crate;", + missing_newline: false + } + ))); } #[test] diff --git a/tests/regressions.rs b/tests/regressions.rs index 03c9657..fa0f314 100644 --- a/tests/regressions.rs +++ b/tests/regressions.rs @@ -1,4 +1,4 @@ -use gitpatch::{File, FileMetadata, Hunk, Line, ParseError, Patch, Range}; +use gitpatch::{File, FileMetadata, Hunk, LineKind, ParseError, Patch, Range}; use pretty_assertions::assert_eq; @@ -11,7 +11,7 @@ fn hunk_header_context_is_not_a_line_15() -> Result<(), ParseError<'static>> { x "; let patch = Patch::from_single(sample)?; - assert_eq!(patch.hunks[0].lines, [Line::context("x")]); + assert_eq!(patch.hunks[0].lines, [LineKind::Context.to_line("x")]); Ok(()) } @@ -35,15 +35,18 @@ fn crlf_breaks_stuff_17() -> Result<(), ParseError<'static>> { path: "new.txt".into(), meta: None }, + old_missing_newline: false, + new_missing_newline: false, hunks: vec![Hunk { old_range: Range { start: 0, count: 0 }, new_range: Range { start: 0, count: 0 }, range_hint: "", - lines: vec![Line::context("x")], + old_missing_newline: false, + new_missing_newline: false, + lines: vec![LineKind::Context.to_line("x")], }], } ); - assert!(patch.end_newline_before() && patch.end_newline_after()); Ok(()) } From 0ae89d1b6aa01a9ed5e5f1d4f6895b65518dd4af Mon Sep 17 00:00:00 2001 From: Roman Miklashevskii Date: Wed, 14 May 2025 18:30:08 +0300 Subject: [PATCH 03/13] better test for missing newlines --- tests/parse_patch.rs | 129 +++++++++++++++++++++++-------------------- 1 file changed, 68 insertions(+), 61 deletions(-) diff --git a/tests/parse_patch.rs b/tests/parse_patch.rs index 882c616..6c07072 100644 --- a/tests/parse_patch.rs +++ b/tests/parse_patch.rs @@ -41,71 +41,78 @@ fn test_parse() -> Result<(), ParseError<'static>> { #[test] fn test_parse_no_newline_indicator() -> Result<(), ParseError<'static>> { let sample = "\ ---- before.py -+++ after.py -@@ -1,4 +1,4 @@ --bacon --eggs --ham -+python -+eggy -+hamster - guido -\\ No newline at end of file\n"; - let patch = Patch::from_single(sample)?; - assert_eq!( - patch.old, - File { - path: "before.py".into(), - meta: None - } - ); - assert_eq!( - patch.new, - File { - path: "after.py".into(), - meta: None - } - ); - assert!(patch.old_missing_newline && patch.new_missing_newline); - - assert_eq!(format!("{}\n", patch), sample); - - Ok(()) -} - -#[test] -fn test_parse_no_newline_indicator_inside() -> Result<(), ParseError<'static>> { - let sample = "\ ---- test.txt -+++ test.txt -@@ -1,4 +1,5 @@ - python - eggy - hamster --guido +--- a/bar.txt ++++ b/bar.txt +@@ -1,3 +1,3 @@ + bar + Bar +-BAR ++BAR \\ No newline at end of file -+ -+hello +--- a/baz.txt ++++ b/baz.txt +@@ -1,3 +1,3 @@ + baz + Baz +-BAZ +\\ No newline at end of file ++ZAB +\\ No newline at end of file +--- a/foo.txt ++++ b/foo.txt +@@ -1,3 +1,3 @@ + foo + Foo +-FOO +\\ No newline at end of file ++FOO +--- a/foobar.txt ++++ b/foobar.txt +@@ -1,3 +1,3 @@ + foobar +-FooBar ++BarFoo + FOOBAR \\ No newline at end of file\n"; - let patch = Patch::from_single(sample)?; - assert_eq!( - patch.old, - File { - path: "test.txt".into(), - meta: None - } - ); + let patches = Patch::from_multiple(sample)?; + + assert_eq!(patches.len(), 4); + + assert_eq!(patches[0].old.path, "a/bar.txt"); + assert_eq!(patches[0].new.path, "b/bar.txt"); + assert_eq!(patches[0].hunks.len(), 1); + assert_eq!(patches[0].hunks[0].lines.len(), 4); + assert_eq!(patches[0].old_missing_newline, false); + assert_eq!(patches[0].new_missing_newline, true); + + assert_eq!(patches[1].old.path, "a/baz.txt"); + assert_eq!(patches[1].new.path, "b/baz.txt"); + assert_eq!(patches[1].hunks.len(), 1); + assert_eq!(patches[1].hunks[0].lines.len(), 4); + assert_eq!(patches[1].old_missing_newline, true); + assert_eq!(patches[1].new_missing_newline, true); + + assert_eq!(patches[2].old.path, "a/foo.txt"); + assert_eq!(patches[2].new.path, "b/foo.txt"); + assert_eq!(patches[2].hunks.len(), 1); + assert_eq!(patches[2].hunks[0].lines.len(), 4); + assert_eq!(patches[2].old_missing_newline, true); + assert_eq!(patches[2].new_missing_newline, false); + + assert_eq!(patches[3].old.path, "a/foobar.txt"); + assert_eq!(patches[3].new.path, "b/foobar.txt"); + assert_eq!(patches[3].hunks.len(), 1); + assert_eq!(patches[3].hunks[0].lines.len(), 4); + assert_eq!(patches[3].old_missing_newline, true); + assert_eq!(patches[3].new_missing_newline, true); + assert_eq!( - patch.new, - File { - path: "test.txt".into(), - meta: None - } + patches + .iter() + .map(|patch| format!("{}\n", patch)) + .collect::(), + sample ); - assert!(patch.old_missing_newline && patch.new_missing_newline); - - assert_eq!(format!("{}\n", patch), sample); Ok(()) } From f5b5ab92c757578cee6b3fdb593435f27228647c Mon Sep 17 00:00:00 2001 From: Roman Miklashevskii Date: Wed, 14 May 2025 18:35:19 +0300 Subject: [PATCH 04/13] fix failing test --- src/ast.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ast.rs b/src/ast.rs index 0129049..1ad986a 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -68,7 +68,8 @@ impl<'a> Patch<'a> { /// let patch = Patch::from_single(sample)?; /// assert_eq!(&patch.old.path, "lao"); /// assert_eq!(&patch.new.path, "tzu"); - /// assert_eq!(patch.new_missing_newline && patch.old_missing_newline, false); + /// assert_eq!(patch.old_missing_newline, false); + /// assert_eq!(patch.new_missing_newline, true); /// # Ok(()) /// # } /// ``` From 3970143fd7a8a944070f9fb4d9f59610e8eaab76 Mon Sep 17 00:00:00 2001 From: vrmiguel Date: Thu, 23 Oct 2025 16:55:28 -0300 Subject: [PATCH 05/13] I actually fixed the conflict incorrectly --- src/parser.rs | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index 406677a..ab9925d 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -360,18 +360,7 @@ fn u64_digit(input: Input<'_>) -> IResult, u64> { Ok((input, num)) } -// Trailing newline indicator -fn no_newline_indicator(input: Input<'_>) -> IResult, bool> { - map( - opt(terminated( - tag("\\ No newline at end of file"), - opt(line_ending), - )), - |matched| matched.is_some(), - )(input) -} - -fn filename(input: Input<'_>) -> IResult, Cow> { +fn filename(input: Input<'_>) -> IResult, Cow<'_, str>> { alt((quoted, bare))(input) } From 75a3b699effbca14b538f6b3659b2fce34117c64 Mon Sep 17 00:00:00 2001 From: vrmiguel Date: Thu, 23 Oct 2025 17:00:17 -0300 Subject: [PATCH 06/13] Clippy --- src/ast.rs | 2 +- src/lib.rs | 1 + tests/parse_samples.rs | 2 -- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 1ad986a..483481a 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -338,7 +338,7 @@ mod tests { new_missing_newline: false, lines: vec![], }; - for (input, expected) in vec![ + for (input, expected) in [ ("", None), (" ", None), (" ", None), diff --git a/src/lib.rs b/src/lib.rs index 9fdfd73..7125a2a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -44,6 +44,7 @@ //! [spec]: http://www.artima.com/weblogs/viewpost.jsp?thread=164293 #![deny(unused_must_use)] +#![allow(clippy::format_collect)] mod ast; mod parser; diff --git a/tests/parse_samples.rs b/tests/parse_samples.rs index 50e223a..4a81813 100644 --- a/tests/parse_samples.rs +++ b/tests/parse_samples.rs @@ -20,7 +20,6 @@ fn parse_samples() { // Make sure that the patch file we produce parses to the same information as the original // patch file. - #[allow(clippy::format_collect)] // Display::fmt is the only way to resolve Patch->str let patch_file: String = patches.iter().map(|patch| format!("{}\n", patch)).collect(); println!("{}", patch_file); let patches2 = Patch::from_multiple(&patch_file).unwrap_or_else(|err| { @@ -51,7 +50,6 @@ fn parse_wild_samples() { // Make sure that the patch file we produce parses to the same information as the original // patch file. - #[allow(clippy::format_collect)] // Display::fmt is the only way to resolve Patch->str let patch_file: String = patches.iter().map(|patch| format!("{}\n", patch)).collect(); let patches2 = Patch::from_multiple(&patch_file).unwrap_or_else(|err| { From e01ffbbf1c6fdafe9a8dec5fac8c7d0418de61c1 Mon Sep 17 00:00:00 2001 From: Roman Miklashevskii Date: Mon, 8 Dec 2025 18:06:25 +0100 Subject: [PATCH 07/13] fix newlines in apply example --- examples/apply.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/apply.rs b/examples/apply.rs index 01b0e67..e894001 100644 --- a/examples/apply.rs +++ b/examples/apply.rs @@ -26,7 +26,7 @@ fn apply(diff: Patch, old: &str) -> String { if need_new_line { out.push(""); } - out.join("") + out.join("\n") } static LAO: &str = "\ From 54eda23c15f2aaeac3ce82b4a9040a9306a2d533 Mon Sep 17 00:00:00 2001 From: Roman Miklashevskii Date: Mon, 8 Dec 2025 19:35:53 +0100 Subject: [PATCH 08/13] refactor: only have missing_newline flags in Patch --- examples/apply.rs | 14 ++-- src/ast.rs | 115 ++++++++++++--------------- src/parser.rs | 172 ++++++++++++++++++++--------------------- tests/parse_patch.rs | 14 ++-- tests/parse_samples.rs | 2 +- tests/regressions.rs | 12 ++- 6 files changed, 152 insertions(+), 177 deletions(-) diff --git a/examples/apply.rs b/examples/apply.rs index e894001..d3811e6 100644 --- a/examples/apply.rs +++ b/examples/apply.rs @@ -1,12 +1,11 @@ //! Demonstrates how to apply a parsed diff to a file -use gitpatch::{LineKind, Patch}; +use gitpatch::{Line, Patch}; fn apply(diff: Patch, old: &str) -> String { let old_lines = old.lines().collect::>(); let mut out: Vec<&str> = vec![]; let mut old_line = 0; - let mut need_new_line = false; for hunk in diff.hunks { while old_line < hunk.old_range.start - 1 { out.push(old_lines[old_line as usize]); @@ -14,16 +13,15 @@ fn apply(diff: Patch, old: &str) -> String { } old_line += hunk.old_range.count; for line in hunk.lines { - match line.kind { - LineKind::Add | LineKind::Context => { - out.push(line.content); - need_new_line = !line.missing_newline; + match line { + Line::Add(s) | Line::Context(s) => { + out.push(s); } - LineKind::Remove => {} + Line::Remove(_) => {} } } } - if need_new_line { + if !diff.new_missing_newline { out.push(""); } out.join("\n") diff --git a/src/ast.rs b/src/ast.rs index 483481a..9053310 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -12,12 +12,12 @@ pub struct Patch<'a> { pub old: File<'a>, /// The file information of the `+` side of the diff, line prefix: `+++` pub new: File<'a>, + /// hunks of differences; each hunk shows one area where the files differ + pub hunks: Vec>, /// If there was a `No newline at end of file` indicator after the last line of the old version of the file pub old_missing_newline: bool, /// If there was a `No newline at end of file` indicator after the last line of the new version of the file pub new_missing_newline: bool, - /// hunks of differences; each hunk shows one area where the files differ - pub hunks: Vec>, } impl fmt::Display for Patch<'_> { @@ -27,8 +27,13 @@ impl fmt::Display for Patch<'_> { write!(f, "--- {}", self.old)?; write!(f, "\n+++ {}", self.new)?; - for hunk in &self.hunks { - write!(f, "\n{}", hunk)?; + for (i, hunk) in self.hunks.iter().enumerate() { + writeln!(f)?; + if i == self.hunks.len() - 1 { + hunk.fmt(f, self.old_missing_newline, self.new_missing_newline)?; + } else { + hunk.fmt(f, false, false)?; + } } Ok(()) } @@ -68,8 +73,7 @@ impl<'a> Patch<'a> { /// let patch = Patch::from_single(sample)?; /// assert_eq!(&patch.old.path, "lao"); /// assert_eq!(&patch.new.path, "tzu"); - /// assert_eq!(patch.old_missing_newline, false); - /// assert_eq!(patch.new_missing_newline, true); + /// assert_eq!(patch.end_newline, false); /// # Ok(()) /// # } /// ``` @@ -78,7 +82,9 @@ impl<'a> Patch<'a> { } /// Attempt to parse as many patches as possible from the given string. This is useful for when - /// you have a complete diff of many files. String must contain at least one patch. + /// you have a complete diff of many files. + /// + /// It is not an error if the string contains no patches: this returns an empty vector. /// /// # Example /// @@ -215,10 +221,6 @@ pub struct Hunk<'a> { pub new_range: Range, /// Any trailing text after the hunk's range information pub range_hint: &'a str, - /// If there was a `No newline at end of file` indicator after the last line of the old version of the hunk - pub old_missing_newline: bool, - /// If there was a `No newline at end of file` indicator after the last line of the new version of the hunk - pub new_missing_newline: bool, /// Each line of text in the hunk, prefixed with the type of change it represents pub lines: Vec>, } @@ -233,18 +235,41 @@ impl Hunk<'_> { Some(h) } } -} -impl fmt::Display for Hunk<'_> { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fn fmt( + &self, + f: &mut fmt::Formatter, + old_missing_newline: bool, + new_missing_newline: bool, + ) -> fmt::Result { write!( f, "@@ -{} +{} @@{}", self.old_range, self.new_range, self.range_hint )?; - for line in &self.lines { + // compute line indices to put "No newline at end of file" indicator after + let last_old_idx = old_missing_newline + .then(|| { + self.lines + .iter() + .rposition(|l| matches!(l, Line::Remove(_) | Line::Context(_))) + }) + .flatten(); + let last_new_idx = new_missing_newline + .then(|| { + self.lines + .iter() + .rposition(|l| matches!(l, Line::Add(_) | Line::Context(_))) + }) + .flatten(); + + for (i, line) in self.lines.iter().enumerate() { write!(f, "\n{}", line)?; + if Some(i) == last_old_idx || Some(i) == last_new_idx { + writeln!(f)?; + write!(f, "\\ No newline at end of file")?; + } } Ok(()) @@ -266,60 +291,24 @@ impl fmt::Display for Range { } } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum LineKind { +/// A line of the old file, new file, or both +#[derive(Debug, Clone, Eq, PartialEq)] +pub enum Line<'a> { /// A line added to the old file in the new file - Add, + Add(&'a str), /// A line removed from the old file in the new file - Remove, + Remove(&'a str), /// A line provided for context in the diff (unchanged); from both the old and the new file - Context, -} - -impl LineKind { - pub fn to_line_full(self, line: &str, missing_newline: bool) -> Line<'_> { - Line { - kind: self, - content: line, - missing_newline, - } - } - - pub fn to_line(self, line: &str) -> Line<'_> { - Line { - kind: self, - content: line, - missing_newline: false, - } - } -} - -/// A line of the old file, new file, or both -#[derive(Debug, Clone, Eq, PartialEq)] -pub struct Line<'a> { - /// The kind of the line (added, removed, or context) - pub kind: LineKind, - /// The actual text of the line - pub content: &'a str, - /// If there was a `No newline at end of file` indicator after the line - pub missing_newline: bool, + Context(&'a str), } impl fmt::Display for Line<'_> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let prefix = match self.kind { - LineKind::Add => "+", - LineKind::Remove => "-", - LineKind::Context => " ", - }; - - write!(f, "{}{}", prefix, self.content)?; - - if self.missing_newline { - write!(f, "\n\\ No newline at end of file")?; - }; - - Ok(()) + match self { + Line::Add(line) => write!(f, "+{}", line), + Line::Remove(line) => write!(f, "-{}", line), + Line::Context(line) => write!(f, " {}", line), + } } } @@ -334,8 +323,6 @@ mod tests { old_range: Range { start: 0, count: 0 }, new_range: Range { start: 0, count: 0 }, range_hint: "", - old_missing_newline: false, - new_missing_newline: false, lines: vec![], }; for (input, expected) in [ diff --git a/src/parser.rs b/src/parser.rs index ab9925d..0e6fdcb 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -2,13 +2,12 @@ use std::borrow::Cow; use std::error::Error; use chrono::DateTime; -use nom::combinator::verify; use nom::*; use nom::{ branch::alt, bytes::complete::{is_not, tag, take_until}, character::complete::{char, digit1, line_ending, none_of, not_line_ending, one_of}, - combinator::{map, map_opt, not, opt}, + combinator::{all_consuming, map, map_opt, not, opt, verify}, error::context, multi::{many0, many1}, sequence::{delimited, preceded, terminated, tuple}, @@ -88,20 +87,15 @@ pub(crate) fn parse_single_patch(s: &str) -> Result, ParseError<'_>> { pub(crate) fn parse_multiple_patches(s: &str) -> Result>, ParseError<'_>> { let (remaining_input, patches) = multiple_patches(Input::new(s))?; - // Parser should return an error instead of producing remaining input - if !remaining_input.fragment().is_empty() { - return Err(ParseError { - line: remaining_input.location_line(), - offset: remaining_input.location_offset(), - fragment: remaining_input.fragment(), - kind: nom::error::ErrorKind::Eof, - }); - } + debug_assert!( + remaining_input.fragment().is_empty(), + "all_consuming should not have left over input" + ); Ok(patches) } fn multiple_patches(input: Input) -> IResult> { - many1(patch)(input) + all_consuming(many0(patch))(input) } fn patch(input: Input) -> IResult { @@ -112,15 +106,10 @@ fn patch(input: Input) -> IResult { return Ok(patch); } let (input, files) = headers(input)?; - let (input, hunks) = chunks(input)?; + let (input, (hunks, old_missing_newline, new_missing_newline)) = chunks(input)?; // Ignore trailing empty lines produced by some diff programs let (input, _) = many0(line_ending)(input)?; - let (old_missing_newline, new_missing_newline) = hunks - .last() - .map(|hunk| (hunk.old_missing_newline, hunk.new_missing_newline)) - .unwrap_or((false, false)); - let (old, new) = files; Ok(( input, @@ -236,8 +225,17 @@ fn header_line_content(input: Input) -> IResult { } // Hunks of the file differences -fn chunks(input: Input) -> IResult> { - many1(chunk)(input) +fn chunks(input: Input) -> IResult, bool, bool)> { + let (span, hunks) = many1(chunk)(input)?; + + let (old_missing_newline, new_missing_newline) = hunks + .last() + .map(|(_, old_missing_newline, new_missing_newline)| { + (*old_missing_newline, *new_missing_newline) + }) + .unwrap_or_default(); + let hunks = hunks.into_iter().map(|(hunk, _, _)| hunk).collect(); + Ok((span, (hunks, old_missing_newline, new_missing_newline))) } fn is_next_header(input: Input<'_>) -> bool { @@ -276,7 +274,7 @@ fn is_next_header(input: Input<'_>) -> bool { ///FIXME: Use the ranges in the chunk header to figure out how many chunk lines to parse. Will need /// to figure out how to count in nom more robustly than many1!(). Maybe using switch!()? ///FIXME: The test_parse_triple_plus_minus_hack test will no longer panic when this is fixed. -fn chunk(input: Input) -> IResult { +fn chunk(input: Input) -> IResult { let (input, ranges) = chunk_header(input)?; // Parse chunk lines, using the range information to guide parsing @@ -285,52 +283,49 @@ fn chunk(input: Input) -> IResult { // Detect added lines map( preceded(tuple((char('+'), not(tag("++ ")))), consume_content_line), - |(line, missing_newline)| LineKind::Add.to_line_full(line, missing_newline), + |(line, missing_new_line)| (Line::Add(line), missing_new_line), ), // Detect removed lines map( preceded(tuple((char('-'), not(tag("-- ")))), consume_content_line), - |(line, missing_newline)| LineKind::Remove.to_line_full(line, missing_newline), + |(line, missing_new_line)| (Line::Remove(line), missing_new_line), ), // Detect context lines map( preceded(char(' '), consume_content_line), - |(line, missing_newline)| LineKind::Context.to_line_full(line, missing_newline), + |(line, missing_new_line)| (Line::Context(line), missing_new_line), ), // Handle empty lines within the chunk - map(tag("\n"), |_| LineKind::Context.to_line_full("", false)), + map(tag("\n"), |_| (Line::Context(""), false)), )), // Stop parsing when we detect the next header or have parsed the expected number of lines |_| !is_next_header(input), ))(input)?; - // Track whether the last line for each side of the hunk is missing a final newline - let mut new_missing_newline = false; - let mut old_missing_newline = false; - for line in &lines { - let is_new = matches!(line.kind, LineKind::Add | LineKind::Context); - let is_old = matches!(line.kind, LineKind::Remove | LineKind::Context); + let old_missing_newline = lines + .iter() + .filter(|(line, _)| matches!(line, Line::Remove(_) | Line::Context(_))) + .any(|(_, missing_new_line)| *missing_new_line); + let new_missing_newline = lines + .iter() + .filter(|(line, _)| matches!(line, Line::Add(_) | Line::Context(_))) + .any(|(_, missing_new_line)| *missing_new_line); - if is_new { - new_missing_newline = line.missing_newline; - } - - if is_old { - old_missing_newline = line.missing_newline; - } - } + let lines = lines.into_iter().map(|(line, _)| line).collect(); let (old_range, new_range, range_hint) = ranges; Ok(( input, - Hunk { - old_range, - new_range, - range_hint, - lines, + ( + Hunk { + old_range, + new_range, + range_hint, + lines, + }, old_missing_newline, new_missing_newline, - }, + ), )) } @@ -360,7 +355,7 @@ fn u64_digit(input: Input<'_>) -> IResult, u64> { Ok((input, num)) } -fn filename(input: Input<'_>) -> IResult, Cow<'_, str>> { +fn filename(input: Input) -> IResult> { alt((quoted, bare))(input) } @@ -447,6 +442,12 @@ mod tests { Ok(()) } + #[test] + fn test_empty_input() -> ParseResult<'static, ()> { + test_parser!(multiple_patches("") -> @("", Vec::new())); + Ok(()) + } + #[test] fn test_filename() -> ParseResult<'static, ()> { // bare @@ -601,24 +602,26 @@ mod tests { Therefore let there always be non-being, so we may see their subtlety, And let there always be being,\n"; - let expected = Hunk { - old_range: Range { start: 1, count: 7 }, - new_range: Range { start: 1, count: 6 }, - range_hint: "", - old_missing_newline: false, - new_missing_newline: false, - lines: vec![ - LineKind::Remove.to_line("The Way that can be told of is not the eternal Way;"), - LineKind::Remove.to_line("The name that can be named is not the eternal name."), - LineKind::Context.to_line("The Nameless is the origin of Heaven and Earth;"), - LineKind::Remove.to_line("The Named is the mother of all things."), - LineKind::Add.to_line("The named is the mother of all things."), - LineKind::Add.to_line(""), - LineKind::Context.to_line("Therefore let there always be non-being,"), - LineKind::Context.to_line(" so we may see their subtlety,"), - LineKind::Context.to_line("And let there always be being,"), - ], - }; + let expected = ( + Hunk { + old_range: Range { start: 1, count: 7 }, + new_range: Range { start: 1, count: 6 }, + range_hint: "", + lines: vec![ + Line::Remove("The Way that can be told of is not the eternal Way;"), + Line::Remove("The name that can be named is not the eternal name."), + Line::Context("The Nameless is the origin of Heaven and Earth;"), + Line::Remove("The Named is the mother of all things."), + Line::Add("The named is the mother of all things."), + Line::Add(""), + Line::Context("Therefore let there always be non-being,"), + Line::Context(" so we may see their subtlety,"), + Line::Context("And let there always be being,"), + ], + }, + false, + false, + ); test_parser!(chunk(sample) -> expected); Ok(()) } @@ -660,46 +663,39 @@ mod tests { DateTime::parse_from_rfc3339("2002-02-21T23:30:50.442260588-08:00").unwrap(), )), }, - old_missing_newline: false, - new_missing_newline: false, hunks: vec![ Hunk { old_range: Range { start: 1, count: 7 }, new_range: Range { start: 1, count: 6 }, range_hint: "", - old_missing_newline: false, - new_missing_newline: false, lines: vec![ - LineKind::Remove - .to_line("The Way that can be told of is not the eternal Way;"), - LineKind::Remove - .to_line("The name that can be named is not the eternal name."), - LineKind::Context - .to_line("The Nameless is the origin of Heaven and Earth;"), - LineKind::Remove.to_line("The Named is the mother of all things."), - LineKind::Add.to_line("The named is the mother of all things."), - LineKind::Add.to_line(""), - LineKind::Context.to_line("Therefore let there always be non-being,"), - LineKind::Context.to_line(" so we may see their subtlety,"), - LineKind::Context.to_line("And let there always be being,"), + Line::Remove("The Way that can be told of is not the eternal Way;"), + Line::Remove("The name that can be named is not the eternal name."), + Line::Context("The Nameless is the origin of Heaven and Earth;"), + Line::Remove("The Named is the mother of all things."), + Line::Add("The named is the mother of all things."), + Line::Add(""), + Line::Context("Therefore let there always be non-being,"), + Line::Context(" so we may see their subtlety,"), + Line::Context("And let there always be being,"), ], }, Hunk { old_range: Range { start: 9, count: 3 }, new_range: Range { start: 8, count: 6 }, range_hint: "", - old_missing_newline: false, - new_missing_newline: false, lines: vec![ - LineKind::Context.to_line("The two are the same,"), - LineKind::Context.to_line("But after they are produced,"), - LineKind::Context.to_line(" they have different names."), - LineKind::Add.to_line("They both may be called deep and profound."), - LineKind::Add.to_line("Deeper and more profound,"), - LineKind::Add.to_line("The door of all subtleties!"), + Line::Context("The two are the same,"), + Line::Context("But after they are produced,"), + Line::Context(" they have different names."), + Line::Add("They both may be called deep and profound."), + Line::Add("Deeper and more profound,"), + Line::Add("The door of all subtleties!"), ], }, ], + old_missing_newline: false, + new_missing_newline: false, }; test_parser!(patch(sample) -> expected); diff --git a/tests/parse_patch.rs b/tests/parse_patch.rs index 6c07072..d10362d 100644 --- a/tests/parse_patch.rs +++ b/tests/parse_patch.rs @@ -1,5 +1,5 @@ use chrono::DateTime; -use gitpatch::{File, FileMetadata, Line, LineKind, ParseError, Patch}; +use gitpatch::{File, FileMetadata, Line, ParseError, Patch}; use pretty_assertions::assert_eq; @@ -437,14 +437,10 @@ index d923f10..b47f160 100644 assert_eq!(patches[0].old.path, "a/src/ast.rs"); assert_eq!(patches[0].new.path, "b/src/ast-2.rs"); - assert!(patches[0].hunks[0].lines.iter().any(|line| matches!( - line, - Line { - kind: LineKind::Add, - content: "use new_crate;", - missing_newline: false - } - ))); + assert!(patches[0].hunks[0] + .lines + .iter() + .any(|line| matches!(line, Line::Add("use new_crate;")))); } #[test] diff --git a/tests/parse_samples.rs b/tests/parse_samples.rs index 4a81813..08801b3 100644 --- a/tests/parse_samples.rs +++ b/tests/parse_samples.rs @@ -28,7 +28,7 @@ fn parse_samples() { path, err ) }); - assert_eq!(patches, patches2); + assert_eq!(patches, patches2,); } } diff --git a/tests/regressions.rs b/tests/regressions.rs index fa0f314..f02afb7 100644 --- a/tests/regressions.rs +++ b/tests/regressions.rs @@ -1,4 +1,4 @@ -use gitpatch::{File, FileMetadata, Hunk, LineKind, ParseError, Patch, Range}; +use gitpatch::{File, FileMetadata, Hunk, Line, ParseError, Patch, Range}; use pretty_assertions::assert_eq; @@ -11,7 +11,7 @@ fn hunk_header_context_is_not_a_line_15() -> Result<(), ParseError<'static>> { x "; let patch = Patch::from_single(sample)?; - assert_eq!(patch.hunks[0].lines, [LineKind::Context.to_line("x")]); + assert_eq!(patch.hunks[0].lines, [Line::Context("x")]); Ok(()) } @@ -35,16 +35,14 @@ fn crlf_breaks_stuff_17() -> Result<(), ParseError<'static>> { path: "new.txt".into(), meta: None }, - old_missing_newline: false, - new_missing_newline: false, hunks: vec![Hunk { old_range: Range { start: 0, count: 0 }, new_range: Range { start: 0, count: 0 }, range_hint: "", - old_missing_newline: false, - new_missing_newline: false, - lines: vec![LineKind::Context.to_line("x")], + lines: vec![Line::Context("x")], }], + old_missing_newline: false, + new_missing_newline: false } ); Ok(()) From d155c1f9b966f23900912d3d41f0da491c50f0f7 Mon Sep 17 00:00:00 2001 From: Roman Miklashevskii Date: Mon, 8 Dec 2025 20:35:18 +0100 Subject: [PATCH 09/13] fix all tests. the most interesting thing is about empty trailing lines after the patch. --- src/ast.rs | 3 ++- src/parser.rs | 57 ++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 9053310..ae036c2 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -73,7 +73,8 @@ impl<'a> Patch<'a> { /// let patch = Patch::from_single(sample)?; /// assert_eq!(&patch.old.path, "lao"); /// assert_eq!(&patch.new.path, "tzu"); - /// assert_eq!(patch.end_newline, false); + /// assert_eq!(patch.old_missing_newline, false); + /// assert_eq!(patch.new_missing_newline, true); /// # Ok(()) /// # } /// ``` diff --git a/src/parser.rs b/src/parser.rs index 0e6fdcb..3ae8848 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -277,41 +277,78 @@ fn is_next_header(input: Input<'_>) -> bool { fn chunk(input: Input) -> IResult { let (input, ranges) = chunk_header(input)?; + enum ParsedLine<'a> { + Line(Line<'a>, bool), + EmptyLine, + } + impl<'a> ParsedLine<'a> { + fn is_new(&self) -> bool { + matches!( + self, + Self::EmptyLine | Self::Line(Line::Context(_), _) | Self::Line(Line::Add(_), _) + ) + } + fn is_old(&self) -> bool { + matches!( + self, + Self::EmptyLine | Self::Line(Line::Context(_), _) | Self::Line(Line::Remove(_), _) + ) + } + fn missing_new_line(&self) -> bool { + match self { + Self::Line(_, missing_new_line) => *missing_new_line, + Self::EmptyLine => false, + } + } + fn into_line(self) -> Line<'a> { + match self { + Self::Line(line, _) => line, + Self::EmptyLine => Line::Context(""), + } + } + } + // Parse chunk lines, using the range information to guide parsing - let (input, lines) = many0(verify( + let (input, mut lines) = many0(verify( alt(( // Detect added lines map( preceded(tuple((char('+'), not(tag("++ ")))), consume_content_line), - |(line, missing_new_line)| (Line::Add(line), missing_new_line), + |(line, missing_new_line)| ParsedLine::Line(Line::Add(line), missing_new_line), ), // Detect removed lines map( preceded(tuple((char('-'), not(tag("-- ")))), consume_content_line), - |(line, missing_new_line)| (Line::Remove(line), missing_new_line), + |(line, missing_new_line)| ParsedLine::Line(Line::Remove(line), missing_new_line), ), // Detect context lines map( preceded(char(' '), consume_content_line), - |(line, missing_new_line)| (Line::Context(line), missing_new_line), + |(line, missing_new_line)| ParsedLine::Line(Line::Context(line), missing_new_line), ), // Handle empty lines within the chunk - map(tag("\n"), |_| (Line::Context(""), false)), + map(tag("\n"), |_| ParsedLine::EmptyLine), )), // Stop parsing when we detect the next header or have parsed the expected number of lines |_| !is_next_header(input), ))(input)?; + // remove trailing empty lines + while matches!(lines.last(), Some(ParsedLine::EmptyLine)) { + lines.pop(); + } + + // was there "missing new line" indicator for any of the lines? let old_missing_newline = lines .iter() - .filter(|(line, _)| matches!(line, Line::Remove(_) | Line::Context(_))) - .any(|(_, missing_new_line)| *missing_new_line); + .filter(|line| line.is_old()) + .any(|line| line.missing_new_line()); let new_missing_newline = lines .iter() - .filter(|(line, _)| matches!(line, Line::Add(_) | Line::Context(_))) - .any(|(_, missing_new_line)| *missing_new_line); + .filter(|line| line.is_new()) + .any(|line| line.missing_new_line()); - let lines = lines.into_iter().map(|(line, _)| line).collect(); + let lines = lines.into_iter().map(ParsedLine::into_line).collect(); let (old_range, new_range, range_hint) = ranges; Ok(( From 4643a4b8d0a8bc9b889738eab9f4ceb1a3bfb4b8 Mon Sep 17 00:00:00 2001 From: Roman Miklashevskii Date: Mon, 8 Dec 2025 20:48:13 +0100 Subject: [PATCH 10/13] remove allow(clippy::format_collect) as not needed anymore --- src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 7125a2a..9fdfd73 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -44,7 +44,6 @@ //! [spec]: http://www.artima.com/weblogs/viewpost.jsp?thread=164293 #![deny(unused_must_use)] -#![allow(clippy::format_collect)] mod ast; mod parser; From cedd636a81cd7cca37ce6282cc73a6c65d8dcf1d Mon Sep 17 00:00:00 2001 From: Roman Miklashevskii Date: Mon, 8 Dec 2025 21:05:24 +0100 Subject: [PATCH 11/13] replace tuples with named structs: ParsedHunk, ParsedHunks. rename ParsedLine to LineOrEmptyLine to avoid confusion --- src/parser.rs | 122 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 78 insertions(+), 44 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index 3ae8848..e8e8349 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -106,7 +106,14 @@ fn patch(input: Input) -> IResult { return Ok(patch); } let (input, files) = headers(input)?; - let (input, (hunks, old_missing_newline, new_missing_newline)) = chunks(input)?; + let ( + input, + ParsedHunks { + hunks, + old_missing_newline, + new_missing_newline, + }, + ) = chunks(input)?; // Ignore trailing empty lines produced by some diff programs let (input, _) = many0(line_ending)(input)?; @@ -224,8 +231,14 @@ fn header_line_content(input: Input) -> IResult { )) } +struct ParsedHunks<'a> { + hunks: Vec>, + old_missing_newline: bool, + new_missing_newline: bool, +} + // Hunks of the file differences -fn chunks(input: Input) -> IResult, bool, bool)> { +fn chunks(input: Input) -> IResult { let (span, hunks) = many1(chunk)(input)?; let (old_missing_newline, new_missing_newline) = hunks @@ -235,7 +248,14 @@ fn chunks(input: Input) -> IResult, bool, bool)> { }) .unwrap_or_default(); let hunks = hunks.into_iter().map(|(hunk, _, _)| hunk).collect(); - Ok((span, (hunks, old_missing_newline, new_missing_newline))) + Ok(( + span, + ParsedHunks { + hunks, + old_missing_newline, + new_missing_newline, + }, + )) } fn is_next_header(input: Input<'_>) -> bool { @@ -246,6 +266,47 @@ fn is_next_header(input: Input<'_>) -> bool { || input.starts_with("@@ ") } +struct ParsedHunk<'a> { + hunk: Hunk<'a>, + old_missing_newline: bool, + new_missing_newline: bool, +} + +enum LineOrEmptyLine<'a> { + Line(Line<'a>, bool), + EmptyLine, +} + +impl<'a> LineOrEmptyLine<'a> { + fn is_new(&self) -> bool { + matches!( + self, + Self::EmptyLine | Self::Line(Line::Context(_), _) | Self::Line(Line::Add(_), _) + ) + } + + fn is_old(&self) -> bool { + matches!( + self, + Self::EmptyLine | Self::Line(Line::Context(_), _) | Self::Line(Line::Remove(_), _) + ) + } + + fn missing_new_line(&self) -> bool { + match self { + Self::Line(_, missing_new_line) => *missing_new_line, + Self::EmptyLine => false, + } + } + + fn into_line(self) -> Line<'a> { + match self { + Self::Line(line, _) => line, + Self::EmptyLine => Line::Context(""), + } + } +} + /// Looks for lines starting with + or - or space, but not +++ or ---. Not a foolproof check. /// /// For example, if someone deletes a line that was using the pre-decrement (--) operator or adds a @@ -274,67 +335,40 @@ fn is_next_header(input: Input<'_>) -> bool { ///FIXME: Use the ranges in the chunk header to figure out how many chunk lines to parse. Will need /// to figure out how to count in nom more robustly than many1!(). Maybe using switch!()? ///FIXME: The test_parse_triple_plus_minus_hack test will no longer panic when this is fixed. -fn chunk(input: Input) -> IResult { +fn chunk(input: Input) -> IResult { let (input, ranges) = chunk_header(input)?; - enum ParsedLine<'a> { - Line(Line<'a>, bool), - EmptyLine, - } - impl<'a> ParsedLine<'a> { - fn is_new(&self) -> bool { - matches!( - self, - Self::EmptyLine | Self::Line(Line::Context(_), _) | Self::Line(Line::Add(_), _) - ) - } - fn is_old(&self) -> bool { - matches!( - self, - Self::EmptyLine | Self::Line(Line::Context(_), _) | Self::Line(Line::Remove(_), _) - ) - } - fn missing_new_line(&self) -> bool { - match self { - Self::Line(_, missing_new_line) => *missing_new_line, - Self::EmptyLine => false, - } - } - fn into_line(self) -> Line<'a> { - match self { - Self::Line(line, _) => line, - Self::EmptyLine => Line::Context(""), - } - } - } - // Parse chunk lines, using the range information to guide parsing let (input, mut lines) = many0(verify( alt(( // Detect added lines map( preceded(tuple((char('+'), not(tag("++ ")))), consume_content_line), - |(line, missing_new_line)| ParsedLine::Line(Line::Add(line), missing_new_line), + |(line, missing_new_line)| LineOrEmptyLine::Line(Line::Add(line), missing_new_line), ), // Detect removed lines map( preceded(tuple((char('-'), not(tag("-- ")))), consume_content_line), - |(line, missing_new_line)| ParsedLine::Line(Line::Remove(line), missing_new_line), + |(line, missing_new_line)| { + LineOrEmptyLine::Line(Line::Remove(line), missing_new_line) + }, ), // Detect context lines map( preceded(char(' '), consume_content_line), - |(line, missing_new_line)| ParsedLine::Line(Line::Context(line), missing_new_line), + |(line, missing_new_line)| { + LineOrEmptyLine::Line(Line::Context(line), missing_new_line) + }, ), // Handle empty lines within the chunk - map(tag("\n"), |_| ParsedLine::EmptyLine), + map(tag("\n"), |_| LineOrEmptyLine::EmptyLine), )), // Stop parsing when we detect the next header or have parsed the expected number of lines |_| !is_next_header(input), ))(input)?; // remove trailing empty lines - while matches!(lines.last(), Some(ParsedLine::EmptyLine)) { + while matches!(lines.last(), Some(LineOrEmptyLine::EmptyLine)) { lines.pop(); } @@ -348,13 +382,13 @@ fn chunk(input: Input) -> IResult { .filter(|line| line.is_new()) .any(|line| line.missing_new_line()); - let lines = lines.into_iter().map(ParsedLine::into_line).collect(); + let lines = lines.into_iter().map(LineOrEmptyLine::into_line).collect(); let (old_range, new_range, range_hint) = ranges; Ok(( input, - ( - Hunk { + ParsedHunk { + hunk: Hunk { old_range, new_range, range_hint, @@ -362,7 +396,7 @@ fn chunk(input: Input) -> IResult { }, old_missing_newline, new_missing_newline, - ), + }, )) } From 60ddb5417c043612bb6519e06098b11b3ec358c4 Mon Sep 17 00:00:00 2001 From: Roman Miklashevskii Date: Mon, 8 Dec 2025 21:15:23 +0100 Subject: [PATCH 12/13] push changes left behind in the previous commit --- src/parser.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index e8e8349..2ca42c7 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -243,11 +243,9 @@ fn chunks(input: Input) -> IResult { let (old_missing_newline, new_missing_newline) = hunks .last() - .map(|(_, old_missing_newline, new_missing_newline)| { - (*old_missing_newline, *new_missing_newline) - }) + .map(|hunk| (hunk.old_missing_newline, hunk.new_missing_newline)) .unwrap_or_default(); - let hunks = hunks.into_iter().map(|(hunk, _, _)| hunk).collect(); + let hunks = hunks.into_iter().map(|hunk| hunk.hunk).collect(); Ok(( span, ParsedHunks { @@ -266,6 +264,7 @@ fn is_next_header(input: Input<'_>) -> bool { || input.starts_with("@@ ") } +#[derive(Debug, PartialEq)] struct ParsedHunk<'a> { hunk: Hunk<'a>, old_missing_newline: bool, @@ -673,8 +672,8 @@ mod tests { Therefore let there always be non-being, so we may see their subtlety, And let there always be being,\n"; - let expected = ( - Hunk { + let expected = ParsedHunk { + hunk: Hunk { old_range: Range { start: 1, count: 7 }, new_range: Range { start: 1, count: 6 }, range_hint: "", @@ -690,9 +689,9 @@ mod tests { Line::Context("And let there always be being,"), ], }, - false, - false, - ); + old_missing_newline: false, + new_missing_newline: false, + }; test_parser!(chunk(sample) -> expected); Ok(()) } From bfa7c6fa7755c7845fb457172519db74e376e7cd Mon Sep 17 00:00:00 2001 From: Roman Miklashevskii Date: Wed, 10 Dec 2025 17:12:28 +0100 Subject: [PATCH 13/13] update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a43dde0..79cb37e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ - `Patch::from_multiple` no longer returns an error on an input that contains no patches, including an empty string. It instead returns an empty vector. +### Fixed +- Issue #4: Fixed parsing of “No newline at end of file” markers so they are recognized even when not the final line of a hunk. + ### Changed ## [v0.7]