diff --git a/CHANGELOG.md b/CHANGELOG.md index a24b301b6..e124fab51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ Improved parsing of qualified constructors in Haskell. +### Diffing + +Improved handling of delimiters ("nested sliders") in language that +prefer the outer delimiter, such as JSON and Lisps. + ## 0.48 (released 12th July 2023) ### Parsing diff --git a/sample_files/compare.expected b/sample_files/compare.expected index 3e6c22970..fcb9977e8 100644 --- a/sample_files/compare.expected +++ b/sample_files/compare.expected @@ -136,6 +136,9 @@ sample_files/multiline_string_before.ml sample_files/multiline_string_after.ml sample_files/nest_before.rs sample_files/nest_after.rs 59f4e15e83e05c0fa36d03f4b2bb4cf4 - +sample_files/nested_slider_before.el sample_files/nested_slider_after.el +dc30db682f9f17dd2866a7b5f2a9a081 - + sample_files/nested_slider_before.rs sample_files/nested_slider_after.rs 5c3dc3d870cdf182658da6a29650d911 - diff --git a/sample_files/nested_slider_after.el b/sample_files/nested_slider_after.el new file mode 100644 index 000000000..8f6c3bc40 --- /dev/null +++ b/sample_files/nested_slider_after.el @@ -0,0 +1,8 @@ +(defun deadgrep--project-root () + "Guess the project root of the given FILE-PATH." + (let ((root default-directory) + (project (project-current))) + (when project + (setq root (project-root project))) + (when root + (deadgrep--lookup-override root)))) diff --git a/sample_files/nested_slider_before.el b/sample_files/nested_slider_before.el new file mode 100644 index 000000000..2c29a8842 --- /dev/null +++ b/sample_files/nested_slider_before.el @@ -0,0 +1,10 @@ +(defun deadgrep--project-root () + "Guess the project root of the given FILE-PATH." + (let ((root default-directory) + (project (project-current))) + (when project + (-when-let (roots (project-roots project)) + (setq root (car roots)))) + (when root + (deadgrep--lookup-override root)))) + diff --git a/src/diff/sliders.rs b/src/diff/sliders.rs index 4d9c0e153..f581cc840 100644 --- a/src/diff/sliders.rs +++ b/src/diff/sliders.rs @@ -122,20 +122,21 @@ fn fix_nested_slider_prefer_outer<'a>(node: &'a Syntax<'a>, change_map: &mut Cha .expect("Changes should be set before slider correction") { Unchanged(_) => { - // All children should be novel except one descendant. - let mut found_unchanged = vec![]; - unchanged_descendants_ignore_delim(children, &mut found_unchanged, change_map); - - if let [unchanged] = found_unchanged[..] { - if matches!(unchanged, List { .. }) - && matches!(change_map.get(unchanged), Some(Novel)) + let mut candidates = vec![]; + unchanged_descendants_for_outer_slider(children, &mut candidates, change_map); + + // We can slide if there is a single unchanged + // descendant, that descendant is a list, and that + // list has novel delimiters. + if let [candidate] = candidates[..] { + if matches!(candidate, List { .. }) + && matches!(change_map.get(candidate), Some(Novel)) { - push_unchanged_to_descendant(node, unchanged, change_map); + push_unchanged_to_descendant(node, candidate, change_map); } } } - ReplacedComment(_, _) | ReplacedString(_, _) => {} - Novel => {} + ReplacedComment(_, _) | ReplacedString(_, _) | Novel => {} } for child in children { @@ -197,9 +198,18 @@ fn unchanged_descendants<'a>( } } -/// Find the descendants of `nodes` that are unchanged, but ignore the -/// delimiter on list nodes. -fn unchanged_descendants_ignore_delim<'a>( +/// Nested sliders require a single unchanged descendant whose +/// delimiters we can slide. +/// +/// ``` +/// (old-1 (novel (old-2))) +/// ``` +/// +/// To slide, we want a single list that contains unchanged items but +/// the outer delimiters are novel. +/// +/// Find all the unchanged descendants. +fn unchanged_descendants_for_outer_slider<'a>( nodes: &[&'a Syntax<'a>], found: &mut Vec<&'a Syntax<'a>>, change_map: &ChangeMap<'a>, @@ -216,24 +226,47 @@ fn unchanged_descendants_ignore_delim<'a>( match node { Atom { .. } => { if is_unchanged { + // If there's an unchanged atom descendant, we + // can't slide. Sliding the delimiters requires a + // single list, or we are potentially changing the + // diff semantically. + // + // Add to the found items, but terminate early + // since we'll never slide. found.push(node); + break; } else { - // No problem + // Novel atom. This is fine, we're looking for a + // single unchanged node. } } List { children, .. } => { - let all_children_unchanged = true; - if is_unchanged { - // Outer list is unchanged, not what we wanted. + // This list is unchanged, and the delimiters are + // unchanged. It's an unchanged descendant, but we + // won't be able to slide its delimiters because + // its delimiters are unchanged. + // + // Add to the found items, but terminate early + // since we'll never slide. found.push(node); + break; } else { - // Is changed. - if all_children_unchanged { - // What we're looking for. + // A list whose outer delimiters are novel. + + let has_unchanged_children = children + .iter() + .any(|node| matches!(change_map.get(node), Some(Unchanged(_)))); + if has_unchanged_children { + // The list has unchanged children and novel + // delimiters. This is a candidate for + // sliding. found.push(node); } else { - unchanged_descendants_ignore_delim(children, found, change_map); + // All of the immediate children are novel, + // recurse in case they have descendants that + // are unchanged. + unchanged_descendants_for_outer_slider(children, found, change_map); } } }