diff --git a/CHANGELOG.md b/CHANGELOG.md index 24c05d90c5..07b4d1219b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 to-be-pushed commit has conflicts, or has no description / committer / author set. [#3029](https://github.com/martinvonz/jj/issues/3029) +* Conflicts involving non-empty files that do not end in a newline no longer + look broken when materialized. + [#3968](https://github.com/martinvonz/jj/issues/3968). This is a partial fix, + see also [#3975](https://github.com/martinvonz/jj/issues/3968) which is not + yet fixed. + ## [0.18.0] - 2024-06-05 ### Breaking changes diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index 9d6ff0212f..31feee233f 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,19 @@ 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 = 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 +254,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/src/merge.rs b/lib/src/merge.rs index e16786ee36..9657dc9372 100644 --- a/lib/src/merge.rs +++ b/lib/src/merge.rs @@ -415,6 +415,13 @@ impl FromIterator for MergeBuilder { } } +impl FromIterator for Merge { + fn from_iter>(iter: I) -> Self { + // TODO(ilyagr): Would using Merge::from_vec be better? + MergeBuilder::from_iter(iter).build() + } +} + impl Extend for MergeBuilder { fn extend>(&mut self, iter: I) { self.values.extend(iter) diff --git a/lib/tests/test_conflicts.rs b/lib/tests/test_conflicts.rs index 4d7ee7d790..d24b760fd7 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]