ok/jj
1
0
Fork 0
forked from mirrors/jj

conflicts: replace missing files by empty in materialized conflict

When we materialize modify/delete conflicts, we currently don't
include any context lines. That's because modify/delete conflicts have
only two sides, so there's no common base to compare to. Hunks that
are unchanged on the "modify" side are therefore not considered
conflicting, and since they they don't contribute new changes, they're
simply skipped (here:
3dfedf5814/lib/src/files.rs (L228-L230)).

It seems more useful to instead pretend that the missing side is an
empty file. That way we'll get a conflict in the entire file.

We can still decide later to make e.g. `jj resolve` prompt the user on
modify/delete conflicts just like `hg resolve` does (or maybe it
actually happens earlier there, I don't remember).

Closes #1244.
This commit is contained in:
Martin von Zweigbergk 2023-02-17 10:05:47 -08:00 committed by Martin von Zweigbergk
parent e1d71c3713
commit e48ace56d1
5 changed files with 29 additions and 13 deletions

View file

@ -13,6 +13,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed bugs
* Modify/delete conflicts now include context lines
[#1244](https://github.com/martinvonz/jj/issues/1244).
## [0.7.0] - 2023-02-16
### Breaking changes

View file

@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use std::cmp::max;
use std::io::{Cursor, Write};
use itertools::Itertools;
@ -154,14 +155,19 @@ pub fn extract_file_conflict_as_single_hunk(
if file_adds.len() != conflict.adds.len() || file_removes.len() != conflict.removes.len() {
return None;
}
let added_content = file_adds
let mut added_content = file_adds
.iter()
.map(|part| get_file_contents(store, path, part))
.collect_vec();
let removed_content = file_removes
let mut removed_content = file_removes
.iter()
.map(|part| get_file_contents(store, path, part))
.collect_vec();
// If the conflict had removed the file on one side, we pretend that the file
// was empty there.
let l = max(added_content.len(), removed_content.len() + 1);
added_content.resize(l, vec![]);
removed_content.resize(l - 1, vec![]);
Some(ConflictHunk {
removes: removed_content,

View file

@ -197,12 +197,16 @@ line 5
removes: vec![file_conflict_part(&base_id)],
adds: vec![file_conflict_part(&modified_id)],
};
// TODO: THis should have context around the conflict (#1244)
insta::assert_snapshot!(&materialize_conflict_string(store, &path, &conflict), @r###"
<<<<<<<
%%%%%%%
line 1
line 2
-line 3
+modified
line 4
line 5
+++++++
>>>>>>>
"###
);

View file

@ -49,8 +49,10 @@ fn test_obslog_with_or_without_diff() {
Resolved conflict in file1:
1 1: <<<<<<<resolved
2 : %%%%%%%
3 : +bar
4 : >>>>>>>
3 : foo
4 : +bar
5 : +++++++
6 : >>>>>>>
o rlvkpnrzqnoo test.user@example.com 2001-02-03 04:05:09.000 +07:00 af536e5af67e conflict
my description
o rlvkpnrzqnoo test.user@example.com 2001-02-03 04:05:09.000 +07:00 6fbba7bcb590
@ -86,10 +88,12 @@ fn test_obslog_with_or_without_diff() {
index e155302a24...2ab19ae607 100644
--- a/file1
+++ b/file1
@@ -1,4 +1,1 @@
@@ -1,6 +1,1 @@
-<<<<<<<
-%%%%%%%
- foo
-+bar
-+++++++
->>>>>>>
+resolved
rlvkpnrzqnoo test.user@example.com 2001-02-03 04:05:09.000 +07:00 af536e5af67e conflict

View file

@ -356,8 +356,8 @@ fn test_baseless_conflict_input_files() {
std::fs::read_to_string(repo_path.join("file")).unwrap()
, @r###"
<<<<<<<
+++++++
a
%%%%%%%
+a
+++++++
b
>>>>>>>
@ -427,15 +427,14 @@ fn test_edit_delete_conflict_input_files() {
<<<<<<<
%%%%%%%
-base
+a
+++++++
a
>>>>>>>
"###);
check_resolve_produces_input_file(&mut test_env, &repo_path, "base", "base\n");
check_resolve_produces_input_file(&mut test_env, &repo_path, "left", "");
// Note that `a` ended up in "right" rather than "left". It's unclear if this
// can or should be fixed.
check_resolve_produces_input_file(&mut test_env, &repo_path, "right", "a\n");
check_resolve_produces_input_file(&mut test_env, &repo_path, "left", "a\n");
check_resolve_produces_input_file(&mut test_env, &repo_path, "right", "");
}
#[test]