Skip to content

Commit ada5a88

Browse files
bors[bot]Veykril
andauthored
Merge #6512
6512: Don't replace parent node when inserting as first child in algo::diff r=SomeoneToIgnore a=Veykril This makes the diff a bit more detailed. See #6287 (comment) for context cc @SomeoneToIgnore Co-authored-by: Lukas Wirth <[email protected]>
2 parents f5f3374 + a61bb4a commit ada5a88

File tree

1 file changed

+89
-36
lines changed

1 file changed

+89
-36
lines changed

crates/syntax/src/algo.rs

Lines changed: 89 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -111,18 +111,28 @@ pub enum InsertPosition<T> {
111111

112112
type FxIndexMap<K, V> = IndexMap<K, V, BuildHasherDefault<rustc_hash::FxHasher>>;
113113

114+
#[derive(Debug, Hash, PartialEq, Eq)]
115+
enum TreeDiffInsertPos {
116+
After(SyntaxElement),
117+
AsFirstChild(SyntaxElement),
118+
}
119+
114120
#[derive(Debug)]
115121
pub struct TreeDiff {
116122
replacements: FxHashMap<SyntaxElement, SyntaxElement>,
117123
deletions: Vec<SyntaxElement>,
118124
// the vec as well as the indexmap are both here to preserve order
119-
insertions: FxIndexMap<SyntaxElement, Vec<SyntaxElement>>,
125+
insertions: FxIndexMap<TreeDiffInsertPos, Vec<SyntaxElement>>,
120126
}
121127

122128
impl TreeDiff {
123129
pub fn into_text_edit(&self, builder: &mut TextEditBuilder) {
124130
for (anchor, to) in self.insertions.iter() {
125-
to.iter().for_each(|to| builder.insert(anchor.text_range().end(), to.to_string()));
131+
let offset = match anchor {
132+
TreeDiffInsertPos::After(it) => it.text_range().end(),
133+
TreeDiffInsertPos::AsFirstChild(it) => it.text_range().start(),
134+
};
135+
to.iter().for_each(|to| builder.insert(offset, to.to_string()));
126136
}
127137
for (from, to) in self.replacements.iter() {
128138
builder.replace(from.text_range(), to.to_string())
@@ -188,19 +198,20 @@ pub fn diff(from: &SyntaxNode, to: &SyntaxNode) -> TreeDiff {
188198
let lhs_child = lhs_children.next();
189199
match (lhs_child.clone(), rhs_children.next()) {
190200
(None, None) => break,
191-
(None, Some(element)) => match last_lhs.clone() {
192-
Some(prev) => {
193-
mark::hit!(diff_insert);
194-
diff.insertions.entry(prev).or_insert_with(Vec::new).push(element);
195-
}
196-
// first iteration, this means we got no anchor element to insert after
197-
// therefor replace the parent node instead
198-
None => {
199-
mark::hit!(diff_replace_parent);
200-
diff.replacements.insert(lhs.clone().into(), rhs.clone().into());
201-
break;
202-
}
203-
},
201+
(None, Some(element)) => {
202+
let insert_pos = match last_lhs.clone() {
203+
Some(prev) => {
204+
mark::hit!(diff_insert);
205+
TreeDiffInsertPos::After(prev)
206+
}
207+
// first iteration, insert into out parent as the first child
208+
None => {
209+
mark::hit!(diff_insert_as_first_child);
210+
TreeDiffInsertPos::AsFirstChild(lhs.clone().into())
211+
}
212+
};
213+
diff.insertions.entry(insert_pos).or_insert_with(Vec::new).push(element);
214+
}
204215
(Some(element), None) => {
205216
mark::hit!(diff_delete);
206217
diff.deletions.push(element);
@@ -224,8 +235,15 @@ pub fn diff(from: &SyntaxNode, to: &SyntaxNode) -> TreeDiff {
224235
}
225236
}
226237
let drain = look_ahead_scratch.drain(..);
227-
if let Some(prev) = last_lhs.clone().filter(|_| insert) {
228-
diff.insertions.entry(prev).or_insert_with(Vec::new).extend(drain);
238+
if insert {
239+
let insert_pos = if let Some(prev) = last_lhs.clone().filter(|_| insert) {
240+
TreeDiffInsertPos::After(prev)
241+
} else {
242+
mark::hit!(insert_first_child);
243+
TreeDiffInsertPos::AsFirstChild(lhs.clone().into())
244+
};
245+
246+
diff.insertions.entry(insert_pos).or_insert_with(Vec::new).extend(drain);
229247
rhs_children = rhs_children_clone;
230248
} else {
231249
go(diff, lhs_ele, rhs_ele)
@@ -632,18 +650,19 @@ mod tests {
632650

633651
#[test]
634652
fn replace_parent() {
635-
mark::check!(diff_replace_parent);
653+
mark::check!(diff_insert_as_first_child);
636654
check_diff(
637655
r#""#,
638656
r#"use foo::bar;"#,
639657
expect![[r#"
640658
insertions:
641659
642-
660+
Line 0: AsFirstChild(Node([email protected]))
661+
-> use foo::bar;
643662
644663
replacements:
645664
646-
Line 0: Node([email protected]) -> use foo::bar;
665+
647666
648667
deletions:
649668
@@ -666,7 +685,7 @@ use baz;"#,
666685
expect![[r#"
667686
insertions:
668687
669-
Line 2: Node([email protected])
688+
Line 2: After(Node([email protected]))
670689
-> "\n"
671690
-> use baz;
672691
@@ -694,7 +713,7 @@ use baz;"#,
694713
expect![[r#"
695714
insertions:
696715
697-
Line 2: Token([email protected] "\n")
716+
Line 2: After(Token([email protected] "\n"))
698717
-> use bar;
699718
-> "\n"
700719
@@ -722,7 +741,7 @@ use baz;"#,
722741
expect![[r#"
723742
insertions:
724743
725-
Line 0: Token([email protected] "\n")
744+
Line 0: After(Token([email protected] "\n"))
726745
-> use foo;
727746
-> "\n"
728747
@@ -737,6 +756,36 @@ use baz;"#,
737756
)
738757
}
739758

759+
#[test]
760+
fn first_child_insertion() {
761+
mark::check!(insert_first_child);
762+
check_diff(
763+
r#"fn main() {
764+
stdi
765+
}"#,
766+
r#"use foo::bar;
767+
768+
fn main() {
769+
stdi
770+
}"#,
771+
expect![[r#"
772+
insertions:
773+
774+
Line 0: AsFirstChild(Node([email protected]))
775+
-> use foo::bar;
776+
-> "\n\n "
777+
778+
replacements:
779+
780+
781+
782+
deletions:
783+
784+
785+
"#]],
786+
);
787+
}
788+
740789
#[test]
741790
fn delete_last() {
742791
mark::check!(diff_delete);
@@ -779,7 +828,7 @@ use crate::AstNode;
779828
expect![[r#"
780829
insertions:
781830
782-
Line 1: Node([email protected])
831+
Line 1: After(Node([email protected]))
783832
-> "\n\n"
784833
-> use crate::AstNode;
785834
@@ -845,10 +894,10 @@ use std::ops::{self, RangeInclusive};
845894
expect![[r#"
846895
insertions:
847896
848-
Line 2: Node([email protected])
897+
Line 2: After(Node([email protected]))
849898
-> ::
850899
-> fmt
851-
Line 6: Token([email protected] "\n")
900+
Line 6: After(Token([email protected] "\n"))
852901
-> use std::hash::BuildHasherDefault;
853902
-> "\n"
854903
-> use std::ops::{self, RangeInclusive};
@@ -892,14 +941,14 @@ fn main() {
892941
expect![[r#"
893942
insertions:
894943
895-
Line 3: Node([email protected])
944+
Line 3: After(Node([email protected]))
896945
-> " "
897946
-> match Err(92) {
898947
Ok(it) => it,
899948
_ => return,
900949
}
901950
-> ;
902-
Line 3: Node([email protected])
951+
Line 3: After(Node([email protected]))
903952
-> "\n "
904953
-> foo(x);
905954
@@ -934,14 +983,18 @@ fn main() {
934983
_ => format!("{}", syn),
935984
};
936985

937-
let insertions = diff.insertions.iter().format_with("\n", |(k, v), f| {
938-
f(&format!(
939-
"Line {}: {:?}\n-> {}",
940-
line_number(k),
941-
k,
942-
v.iter().format_with("\n-> ", |v, f| f(&fmt_syntax(v)))
943-
))
944-
});
986+
let insertions =
987+
diff.insertions.iter().format_with("\n", |(k, v), f| -> Result<(), std::fmt::Error> {
988+
f(&format!(
989+
"Line {}: {:?}\n-> {}",
990+
line_number(match k {
991+
super::TreeDiffInsertPos::After(syn) => syn,
992+
super::TreeDiffInsertPos::AsFirstChild(syn) => syn,
993+
}),
994+
k,
995+
v.iter().format_with("\n-> ", |v, f| f(&fmt_syntax(v)))
996+
))
997+
});
945998

946999
let replacements = diff
9471000
.replacements

0 commit comments

Comments
 (0)