From 8c941fd330fa63988653031a3f062494822ba576 Mon Sep 17 00:00:00 2001 From: Ilya Grigoriev Date: Tue, 25 Jun 2024 22:20:18 -0700 Subject: [PATCH] conflicts: insert trailing newline when materializing conflicts This happens for non-empty files only; the materialization code can handle files and hunks with 0 or more lines, but each line should terminate in a newline. This is an imperfect fix to #3968, a better fix is TODO; see #3975. --- lib/src/conflicts.rs | 18 ++++++++++++++++++ lib/tests/test_conflicts.rs | 24 ++++++++++++++++++++---- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index 9d6ff0212f8..8eb084086bf 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -53,6 +53,8 @@ static CONFLICT_MARKER_REGEX: once_cell::sync::Lazy = once_cell::sync::La .unwrap() }); +/// This function assumes that each hunk consists of 0 or more lines, each +/// ending with a newline. fn write_diff_hunks(hunks: &[DiffHunk], file: &mut dyn Write) -> std::io::Result<()> { for hunk in hunks { match hunk { @@ -218,6 +220,21 @@ pub fn materialize_merge_result( single_hunk: Merge, output: &mut dyn Write, ) -> std::io::Result<()> { + // The following code assumes that the file contains 0 or more lines, each + // ending with `\n`. For now, we force this to be true. + // TODO(#3795): A better solution for this. + let single_hunk: Merge = Merge::from_vec( + single_hunk + .into_iter() + .map(|ContentHunk(mut side)| { + if !side.is_empty() && !side.ends_with(b"\n") { + side.push(b'\n'); + }; + ContentHunk(side) + }) + .collect::>(), + ); + let slices = single_hunk.map(|content| content.0.as_slice()); let merge_result = files::merge(&slices); match merge_result { @@ -239,6 +256,7 @@ pub fn materialize_merge_result( output.write_all( format!(" Conflict {conflict_index} of {num_conflicts}\n").as_bytes(), )?; + let mut add_index = 0; for (base_index, left) in hunk.removes().enumerate() { // The vast majority of conflicts one actually tries to diff --git a/lib/tests/test_conflicts.rs b/lib/tests/test_conflicts.rs index 4d7ee7d790a..d24b760fd71 100644 --- a/lib/tests/test_conflicts.rs +++ b/lib/tests/test_conflicts.rs @@ -357,15 +357,31 @@ fn test_materialize_conflict_no_newlines_at_eof() { @r###" <<<<<<< Conflict 1 of 1 %%%%%%% Changes from base to side #1 - -base+++++++ Contents of side #2 - right>>>>>>> Conflict 1 of 1 ends + -base + +++++++ Contents of side #2 + right + >>>>>>> Conflict 1 of 1 ends "### ); - // BUG(#3968): These conflict markers cannot be parsed + // These conflict markers can be parsed (#3968) + // TODO(#3975): BUG: The result of the parsing has newlines where original files + // didn't. insta::assert_debug_snapshot!(parse_conflict( materialized.as_bytes(), conflict.num_sides() - ),@"None"); + ),@r###" + Some( + [ + Conflicted( + [ + "", + "base\n", + "right\n", + ], + ), + ], + ) + "###); } #[test]