mirrors/jj
1
0
Fork 1
mirror of https://github.com/martinvonz/jj.git synced 2025-01-02 18:01:05 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

conflicts: remove ConflictId from update_conflict_from_content()

Browse source 
For tree-level conflicts (#1624), I plan to remove `ConflictId`
completely. This commit removes `ConflictId` from
`update_conflict_from_content()` by instead making it take a
`Conflict<Option<TreeValue>>` and return a possibly different such
value.

I made the call site in `working_copy` avoid writing the conflict to
the store if it's unchanged, but I didn't make the same optimization
in `merge_tools` becuase it's much more likely to have changed there.
This commit is contained in:
Martin von Zweigbergk 2023-06-10 14:47:32 -07:00 committed by Martin von Zweigbergk
parent 096538ba18
commit 19fd8a917a
4 changed files with 38 additions and 38 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
lib/src/conflicts.rs
View file

@ -17,7 +17,7 @@ use std::io::Write;
use itertools::Itertools;
use crate::backend::{BackendResult, ConflictId, FileId, ObjectId, TreeValue};
use crate::backend::{BackendResult, FileId, ObjectId, TreeValue};
use crate::diff::{find_line_ranges, Diff, DiffHunk};
use crate::files::{ConflictHunk, MergeHunk, MergeResult};
use crate::repo_path::RepoPath;
@ -451,10 +451,9 @@ fn parse_conflict_hunk(input: &[u8]) -> MergeHunk {
pub fn update_conflict_from_content(
store: &Store,
path: &RepoPath,
conflict_id: &ConflictId,
conflict: &Conflict<Option<TreeValue>>,
content: &[u8],
) -> BackendResult<Option<ConflictId>> {
let mut conflict = store.read_conflict(path, conflict_id)?;
) -> BackendResult<Option<Conflict<Option<TreeValue>>>> {
// TODO: Check that the conflict only involves files and convert it to a
// `Conflict<Option<FileId>>` so we can remove the wildcard pattern in the loops
// further down.
@ -465,9 +464,9 @@ pub fn update_conflict_from_content(
// conflicts (for example) are not converted to regular files in the working
// copy.
let mut old_content = Vec::with_capacity(content.len());
materialize_conflict(store, path, &conflict, &mut old_content).unwrap();
materialize_conflict(store, path, conflict, &mut old_content).unwrap();
if content == old_content {
return Ok(Some(conflict_id.clone()));
return Ok(Some(conflict.clone()));
}
let mut removed_content = vec![vec![]; conflict.removes().len()];
@ -504,6 +503,7 @@ pub fn update_conflict_from_content(
// Now write the new files contents we found by parsing the file
// with conflict markers. Update the Conflict object with the new
// FileIds.
let mut new_removes = vec![];
for (i, buf) in removed_content.iter().enumerate() {
match &conflict.removes()[i] {
Some(TreeValue::File { id: _, executable }) => {
@ -512,11 +512,12 @@ pub fn update_conflict_from_content(
id: file_id,
executable: *executable,
};
conflict.set_remove(i, Some(new_value));
new_removes.push(Some(new_value));
}
None if buf.is_empty() => {
// The missing side of a conflict is still represented by
// the empty string we materialized it as => nothing to do
// the empty string we materialized it as
new_removes.push(None);
}
_ => {
// The user edited a non-file side. This should never happen. We consider the
@ -525,6 +526,7 @@ pub fn update_conflict_from_content(
}
}
}
let mut new_adds = vec![];
for (i, buf) in added_content.iter().enumerate() {
match &conflict.adds()[i] {
Some(TreeValue::File { id: _, executable }) => {
@ -533,11 +535,12 @@ pub fn update_conflict_from_content(
id: file_id,
executable: *executable,
};
conflict.set_add(i, Some(new_value));
new_adds.push(Some(new_value));
}
None if buf.is_empty() => {
// The missing side of a conflict is still represented by
// the empty string we materialized it as => nothing to do
new_adds.push(None);
}
_ => {
// The user edited a non-file side. This should never happen. We consider the
@ -546,8 +549,7 @@ pub fn update_conflict_from_content(
}
}
}
let new_conflict_id = store.write_conflict(path, &conflict)?;
Ok(Some(new_conflict_id))
Ok(Some(Conflict::new(new_removes, new_adds)))
}
#[cfg(test)]

12
lib/src/working_copy.rs
View file

@ -658,17 +658,23 @@ impl TreeState {
let mut file = File::open(&disk_path).unwrap();
let mut content = vec![];
file.read_to_end(&mut content).unwrap();
if let Some(new_conflict_id) = update_conflict_from_content(
let conflict = self.store.read_conflict(&repo_path, conflict_id)?;
if let Some(new_conflict) = update_conflict_from_content(
self.store.as_ref(),
&repo_path,
conflict_id,
&conflict,
&content,
)
.unwrap()
{
new_file_state.file_type = FileType::Conflict;
*current_file_state = new_file_state;
tree_builder.set(repo_path, TreeValue::Conflict(new_conflict_id));
if new_conflict != conflict {
let new_conflict_id =
self.store.write_conflict(&repo_path, &new_conflict)?;
tree_builder
.set(repo_path, TreeValue::Conflict(new_conflict_id));
};
return Ok(());
}
}

35
lib/tests/test_conflicts.rs
View file

@ -644,36 +644,30 @@ fn test_update_conflict_from_content() {
Some(file_value(&right_file_id)),
],
);
let conflict_id = store.write_conflict(&path, &conflict).unwrap();
// If the content is unchanged compared to the materialized value, we get the
// old conflict id back.
let mut materialized = vec![];
materialize_conflict(store, &path, &conflict, &mut materialized).unwrap();
let result = update_conflict_from_content(store, &path, &conflict_id, &materialized).unwrap();
assert_eq!(result, Some(conflict_id.clone()));
let result = update_conflict_from_content(store, &path, &conflict, &materialized).unwrap();
assert_eq!(result, Some(conflict.clone()));
// If the conflict is resolved, we get None back to indicate that.
let result = update_conflict_from_content(
store,
&path,
&conflict_id,
b"resolved 1\nline 2\nresolved 3\n",
)
.unwrap();
let result =
update_conflict_from_content(store, &path, &conflict, b"resolved 1\nline 2\nresolved 3\n")
.unwrap();
assert_eq!(result, None);
// If the conflict is partially resolved, we get a new conflict back.
let result = update_conflict_from_content(
store,
&path,
&conflict_id,
&conflict,
b"resolved 1\nline 2\n<<<<<<<\n%%%%%%%\n-line 3\n+left 3\n+++++++\nright 3\n>>>>>>>\n",
)
.unwrap();
assert_ne!(result, None);
assert_ne!(result, Some(conflict_id));
let new_conflict = store.read_conflict(&path, &result.unwrap()).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");
let new_left_file_id = testutils::write_file(store, &path, "resolved 1\nline 2\nleft 3\n");
@ -702,30 +696,27 @@ fn test_update_conflict_from_content_modify_delete() {
vec![Some(file_value(&before_file_id))],
vec![Some(file_value(&after_file_id)), None],
);
let conflict_id = store.write_conflict(&path, &conflict).unwrap();
// If the content is unchanged compared to the materialized value, we get the
// old conflict id back.
let mut materialized = vec![];
materialize_conflict(store, &path, &conflict, &mut materialized).unwrap();
let result = update_conflict_from_content(store, &path, &conflict_id, &materialized).unwrap();
assert_eq!(result, Some(conflict_id.clone()));
let result = update_conflict_from_content(store, &path, &conflict, &materialized).unwrap();
assert_eq!(result, Some(conflict.clone()));
// If the conflict is resolved, we get None back to indicate that.
let result = update_conflict_from_content(store, &path, &conflict_id, b"resolved\n").unwrap();
let result = update_conflict_from_content(store, &path, &conflict, b"resolved\n").unwrap();
assert_eq!(result, None);
// If the conflict is modified, we get a new conflict back.
let result = update_conflict_from_content(
store,
&path,
&conflict_id,
&conflict,
b"<<<<<<<\n%%%%%%%\n line 1\n-line 2 before\n+line 2 modified after\n line 3\n+++++++\n>>>>>>>\n",
)
.unwrap();
assert_ne!(result, None);
assert_ne!(result, Some(conflict_id));
let new_conflict = store.read_conflict(&path, &result.unwrap()).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 =

5
src/merge_tools.rs
View file

@ -246,12 +246,13 @@ pub fn run_mergetool(
let mut new_tree_value: Option<TreeValue> = None;
if editor.merge_tool_edits_conflict_markers {
if let Some(new_conflict_id) = update_conflict_from_content(
if let Some(new_conflict) = update_conflict_from_content(
tree.store(),
repo_path,
&conflict_id,
&conflict,
output_file_contents.as_slice(),
)? {
let new_conflict_id = tree.store().write_conflict(repo_path, &new_conflict)?;
new_tree_value = Some(TreeValue::Conflict(new_conflict_id));
}
}