diff --git a/cli/src/merge_tools.rs b/cli/src/merge_tools.rs index f35649efd..d983c23ec 100644 --- a/cli/src/merge_tools.rs +++ b/cli/src/merge_tools.rs @@ -325,12 +325,13 @@ pub fn run_mergetool( let mut new_tree_value: Option = 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)); diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index a095a61f3..5e70b8a7c 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -294,13 +294,15 @@ fn parse_conflict_hunk(input: &[u8]) -> Merge { 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>, store: &Store, path: &RepoPath, content: &[u8], -) -> BackendResult>>> { +) -> BackendResult>> { // 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)) } diff --git a/lib/src/working_copy.rs b/lib/src/working_copy.rs index 306501cf3..9826bb259 100644 --- a/lib/src/working_copy.rs +++ b/lib/src/working_copy.rs @@ -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)) diff --git a/lib/tests/test_conflicts.rs b/lib/tests/test_conflicts.rs index 8043263f0..190cc9ff2 100644 --- a/lib/tests/test_conflicts.rs +++ b/lib/tests/test_conflicts.rs @@ -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 =