Fix sliders in a single global pass

Previously we fixed sliders in each 'possibly changed' region. This
meant that we couldn't fix sliders that needed to move outside the
region. The most common case was code of the form `foo, bar, baz`
where `, baz` was unchanged but we wanted to slide to `,`.

We now call `fix_all_sliders` for the toplevel tree on both
sides. This required some minor changes to the slider logic, as the
unchanged/novel regions could occur at any level of the tree.

(It was probably also the case that we were missing slider
opportunities previously, because we terminated as soon as we found an
outer slider for the nested case.)

This change has no performance impact, probably because tree diffing
is vastly more expensive (O(N^2)) than sliders (O(N)).

Fixes #327
add_libdifftastic 0.35.0
Wilfred Hughes 2022-09-02 18:10:09 +07:00
parent 114235e4da
commit b104c4be10
5 changed files with 32 additions and 15 deletions

@ -151,6 +151,9 @@ b1fe2c184a9a358e314e21aabb0f3cb7 -
sample_files/simple_before.txt sample_files/simple_after.txt
4b653ebe89321835c35722dd065cf6a2 -
sample_files/slider_at_end_before.json sample_files/slider_at_end_after.json
1d6162aab8e59c4422e9c14f09ceac3e -
sample_files/slider_before.rs sample_files/slider_after.rs
50e1df5af0bf4a1fa7211e079196f1a9 -

@ -0,0 +1,5 @@
[
"one",
"two",
"three"
]

@ -0,0 +1,7 @@
[
"one",
"novel-1",
"two",
"novel-2",
"three"
]

@ -137,11 +137,11 @@ fn fix_nested_slider_prefer_outer<'a>(node: &'a Syntax<'a>, change_map: &mut Cha
}
}
ReplacedComment(_, _) => {}
Novel => {
for child in children {
fix_nested_slider_prefer_outer(child, change_map);
}
}
Novel => {}
}
for child in children {
fix_nested_slider_prefer_outer(child, change_map);
}
}
}
@ -155,11 +155,7 @@ fn fix_nested_slider_prefer_inner<'a>(node: &'a Syntax<'a>, change_map: &mut Cha
.get(node)
.expect("Changes should be set before slider correction")
{
Unchanged(_) => {
for child in children {
fix_nested_slider_prefer_inner(child, change_map);
}
}
Unchanged(_) => {}
ReplacedComment(_, _) => {}
Novel => {
let mut found_unchanged = vec![];
@ -170,6 +166,10 @@ fn fix_nested_slider_prefer_inner<'a>(node: &'a Syntax<'a>, change_map: &mut Cha
}
}
}
for child in children {
fix_nested_slider_prefer_inner(child, change_map);
}
}
}

@ -327,10 +327,6 @@ fn diff_file_content(
break;
}
}
let language = language.unwrap();
fix_all_sliders(language, &lhs_section_nodes, &mut change_map);
fix_all_sliders(language, &rhs_section_nodes, &mut change_map);
}
if exceeded_graph_limit {
@ -342,10 +338,16 @@ fn diff_file_content(
rhs_positions,
)
} else {
// TODO: Make this .expect() unnecessary.
let language =
language.expect("If we had a ts_lang, we must have guessed the language");
fix_all_sliders(language, &lhs, &mut change_map);
fix_all_sliders(language, &rhs, &mut change_map);
let lhs_positions = syntax::change_positions(&lhs, &change_map);
let rhs_positions = syntax::change_positions(&rhs, &change_map);
(
language.map(|l| language_name(l).into()),
Some(language_name(language).into()),
lhs_positions,
rhs_positions,
)