ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

conflicts: make update_from_content() write resolved content to store

Browse source 
`update_from_content()` already writes file content for each term of
an unresolved merge, so it seems consistent for it to also write the
file content for resolved merges. I think this should simplify further
refactoring for tree-level conflicts and for preserving the executable
bit.
This commit is contained in:
Martin von Zweigbergk 2023-08-11 14:48:10 -07:00 committed by Martin von Zweigbergk
parent 0b85f06e3d
commit 4c46398b1c
4 changed files with 39 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
cli/src/merge_tools.rs
View file

@ -325,12 +325,13 @@ pub fn run_mergetool(
let mut new_tree_value: Option<TreeValue> = None;
if editor.merge_tool_edits_conflict_markers {
if let Some(new_file_ids) = conflicts::update_from_content(
let new_file_ids = conflicts::update_from_content(
&file_merge,
tree.store(),
repo_path,
output_file_contents.as_slice(),
)? {
)?;
if !new_file_ids.is_resolved() {
let new_conflict = conflict.with_new_file_ids(&new_file_ids);
let new_conflict_id = tree.store().write_conflict(repo_path, &new_conflict)?;
new_tree_value = Some(TreeValue::Conflict(new_conflict_id));

19
lib/src/conflicts.rs
View file

@ -294,13 +294,15 @@ fn parse_conflict_hunk(input: &[u8]) -> Merge<ContentHunk> {
Merge::new(removes, adds)
}
/// Returns `None` if there are no conflict markers in `content`.
/// Parses conflict markers in `content` and returns an updated version of
/// `file_ids` with the new contents. If no (valid) conflict markers remain, a
/// single resolves `FileId` will be returned.
pub fn update_from_content(
file_ids: &Merge<Option<FileId>>,
store: &Store,
path: &RepoPath,
content: &[u8],
) -> BackendResult<Option<Merge<Option<FileId>>>> {
) -> BackendResult<Merge<Option<FileId>>> {
// First check if the new content is unchanged compared to the old content. If
// it is, we don't need parse the content or write any new objects to the
// store. This is also a way of making sure that unchanged tree/file
@ -310,7 +312,7 @@ pub fn update_from_content(
let merge_hunk = extract_as_single_hunk(file_ids, store, path);
materialize_merge_result(&merge_hunk, &mut old_content).unwrap();
if content == old_content {
return Ok(Some(file_ids.clone()));
return Ok(file_ids.clone());
}
let mut removed_content = vec![vec![]; file_ids.removes().len()];
@ -318,7 +320,8 @@ pub fn update_from_content(
let Some(hunks) = parse_conflict(content, file_ids.removes().len(), file_ids.adds().len())
else {
// Either there are no self markers of they don't have the expected arity
return Ok(None);
let file_id = store.write_file(path, &mut &content[..])?;
return Ok(Merge::normal(file_id));
};
for hunk in hunks {
if let Some(slice) = hunk.as_resolved() {
@ -356,7 +359,8 @@ pub fn update_from_content(
_ => {
// The user edited the empty placeholder for an absent side. We consider the
// conflict resolved.
return Ok(None);
let file_id = store.write_file(path, &mut &content[..])?;
return Ok(Merge::normal(file_id));
}
}
}
@ -375,9 +379,10 @@ pub fn update_from_content(
_ => {
// The user edited the empty placeholder for an absent side. We consider the
// conflict resolved.
return Ok(None);
let file_id = store.write_file(path, &mut &content[..])?;
return Ok(Merge::normal(file_id));
}
}
}
Ok(Some(Merge::new(new_removes, new_adds)))
Ok(Merge::new(new_removes, new_adds))
}

28
lib/src/working_copy.rs
View file

@ -969,24 +969,28 @@ impl TreeState {
file.read_to_end(&mut content).unwrap();
let conflict = self.store.read_conflict(repo_path, &conflict_id)?;
if let Some(old_file_ids) = conflict.to_file_merge() {
if let Some(new_file_ids) = conflicts::update_from_content(
let new_file_ids = conflicts::update_from_content(
&old_file_ids,
self.store.as_ref(),
repo_path,
&content,
)
.unwrap()
{
if new_file_ids != old_file_ids {
let new_conflict = conflict.with_new_file_ids(&new_file_ids);
let new_conflict_id = self.store.write_conflict(repo_path, &new_conflict)?;
Ok(TreeValue::Conflict(new_conflict_id))
} else {
Ok(TreeValue::Conflict(conflict_id))
.unwrap();
match new_file_ids.into_resolved() {
Ok(file_id) => Ok(TreeValue::File {
id: file_id.unwrap(),
executable,
}),
Err(new_file_ids) => {
if new_file_ids != old_file_ids {
let new_conflict = conflict.with_new_file_ids(&new_file_ids);
let new_conflict_id =
self.store.write_conflict(repo_path, &new_conflict)?;
Ok(TreeValue::Conflict(new_conflict_id))
} else {
Ok(TreeValue::Conflict(conflict_id))
}
}
} else {
let id = self.store.write_file(repo_path, &mut content.as_slice())?;
Ok(TreeValue::File { id, executable })
}
} else {
Ok(TreeValue::Conflict(conflict_id))

16
lib/tests/test_conflicts.rs
View file

@ -636,22 +636,22 @@ fn test_update_conflict_from_content() {
// old conflict id back.
let materialized = materialize_conflict_string(store, &path, &conflict);
let result = update_from_content(&conflict, store, &path, materialized.as_bytes()).unwrap();
assert_eq!(result, Some(conflict.clone()));
assert_eq!(result, conflict);
// If the conflict is resolved, we get None back to indicate that.
let result =
update_from_content(&conflict, store, &path, b"resolved 1\nline 2\nresolved 3\n").unwrap();
assert_eq!(result, None);
let expected_file_id = testutils::write_file(store, &path, "resolved 1\nline 2\nresolved 3\n");
assert_eq!(result, Merge::normal(expected_file_id));
// If the conflict is partially resolved, we get a new conflict back.
let result = update_from_content(
let new_conflict = update_from_content(
&conflict,
store,
&path,
b"resolved 1\nline 2\n<<<<<<<\n%%%%%%%\n-line 3\n+left 3\n+++++++\nright 3\n>>>>>>>\n",
)
.unwrap();
let new_conflict = result.unwrap();
assert_ne!(new_conflict, conflict);
// Calculate expected new FileIds
let new_base_file_id = testutils::write_file(store, &path, "resolved 1\nline 2\nline 3\n");
@ -683,20 +683,20 @@ fn test_update_conflict_from_content_modify_delete() {
// old conflict id back.
let materialized = materialize_conflict_string(store, &path, &conflict);
let result = update_from_content(&conflict, store, &path, materialized.as_bytes()).unwrap();
assert_eq!(result, Some(conflict.clone()));
assert_eq!(result, conflict);
// If the conflict is resolved, we get None back to indicate that.
let result = update_from_content(&conflict, store, &path, b"resolved\n").unwrap();
assert_eq!(result, None);
let expected_file_id = testutils::write_file(store, &path, "resolved\n");
assert_eq!(result, Merge::normal(expected_file_id));
// If the conflict is modified, we get a new conflict back.
let result = update_from_content(&conflict,
let new_conflict = update_from_content(&conflict,
store,
&path,
b"<<<<<<<\n%%%%%%%\n line 1\n-line 2 before\n+line 2 modified after\n line 3\n+++++++\n>>>>>>>\n",
)
.unwrap();
let new_conflict = result.unwrap();
// Calculate expected new FileIds
let new_base_file_id = testutils::write_file(store, &path, "line 1\nline 2 before\nline 3\n");
let new_left_file_id =