Skip to content

Commit

Permalink
conflicts: insert trailing newline when materializing conflicts
Browse files Browse the repository at this point in the history
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 jj-vcs#3968, a better fix is TODO; see jj-vcs#3975.

This also implements FromIterator for Merge. I could split it into a
separate commit, but then it'd have no uses.
  • Loading branch information
ilyagr committed Jun 27, 2024
1 parent d1dc97f commit 308b0b7
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 4 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions lib/src/conflicts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ static CONFLICT_MARKER_REGEX: once_cell::sync::Lazy<Regex> = 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 {
Expand Down Expand Up @@ -218,6 +220,19 @@ pub fn materialize_merge_result(
single_hunk: Merge<ContentHunk>,
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<ContentHunk> = 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 {
Expand All @@ -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
Expand Down
7 changes: 7 additions & 0 deletions lib/src/merge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,13 @@ impl<T> FromIterator<T> for MergeBuilder<T> {
}
}

impl<T> FromIterator<T> for Merge<T> {
fn from_iter<I: IntoIterator<Item = T>>(iter: I) -> Self {
// TODO(ilyagr): Would using Merge::from_vec be better?
MergeBuilder::from_iter(iter).build()
}
}

impl<T> Extend<T> for MergeBuilder<T> {
fn extend<I: IntoIterator<Item = T>>(&mut self, iter: I) {
self.values.extend(iter)
Expand Down
24 changes: 20 additions & 4 deletions lib/tests/test_conflicts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down

0 comments on commit 308b0b7

Please sign in to comment.