Rewrite algo::diff to support insertion and deletion #6310
Conversation
| r#" | ||
| fn {a:42, b: ()} {} | ||
| fn some(, b: ()} {} | ||
| fn items() {} |
There was a problem hiding this comment.
Thanks to this macro text expecting already broken output you can see the result of these changes in that the diff became smaller here 😄
crates/syntax/src/algo.rs
Outdated
| deletions: Vec<SyntaxElement>, | ||
| // the vec as well as the indexmap are both here to preserve order | ||
| insertions: | ||
| IndexMap<SyntaxElement, Vec<SyntaxElement>, BuildHasherDefault<rustc_hash::FxHasher>>, |
There was a problem hiding this comment.
Insertion order is important as insertions might be chained after each other. The IndexMap could be replace by Vec<(SyntaxElement, Vec<SyntaxElement>) with binary_search insertion on the first tuple part as the key but I'm uncertain what would be better.
| (Some(lhs_ele), Some(rhs_ele)) => go(diff, lhs_ele, rhs_ele), | ||
| } | ||
| last_lhs = lhs_child.or(last_lhs); |
There was a problem hiding this comment.
Hm, would this work corretly if the go ends up replacing the whole lhs_child, and then, on the next iteration, we use it as an anchor in inserts?
There was a problem hiding this comment.
Ah, it seems it will work, cool!
| // first iteration, this means we got no anchor element to insert after | ||
| // therefor replace the parent node instead |
There was a problem hiding this comment.
An alternative is to key insertion not by SyntaxElement, but by (SyntaxElement, bool), where bool should choose between prev sibling and parent. But this is also ok
|
Nifty, didn't realize that we can improve tree diffing this way! r=me wiith FxIndexMap nit fixed If you feel like ii though, adding some unit-tests for tree diff based on |
Veykril
force-pushed
the
algo-diff-rewrite
branch
2 times, most recently
from
ef08d2f to
32fc764
Compare
Veykril
force-pushed
the
algo-diff-rewrite
branch
from
32fc764 to
0059188
Compare
|
bors r+ |
3 participants
This in turn also makes
algo::diffgenerate finer diffs(maybe even minimal diffs?) as insertions and deletions aren't always represented as as replacements of parent nodes now.Required for #6287 to go on.