mirror of
https://github.com/zed-industries/zed.git
synced 2026-05-30 20:24:08 +00:00
Fix an ordering problem that led to invalid edits in display map sync (#52930)
Co-authored-by: Cole Miller <cole@zed.dev> Self-Review Checklist: - [ ] I've reviewed my own diff for quality, security, and reliability - [ ] Unsafe blocks (if any) have justifying comments - [ ] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [ ] Tests cover the new/changed behavior - [ ] Performance impact has been considered and is acceptable Fixes ZED-67X Fixes ZED-50E Release Notes: - N/A Co-authored-by: Cole Miller <cole@zed.dev>
This commit is contained in:
parent
60d6e13832
commit
d6fe14b3cc
1 changed files with 65 additions and 2 deletions
|
|
@ -400,10 +400,13 @@ impl DisplayMap {
|
|||
diagnostics_max_severity: DiagnosticSeverity,
|
||||
cx: &mut Context<Self>,
|
||||
) -> Self {
|
||||
let buffer_subscription = buffer.update(cx, |buffer, _| buffer.subscribe());
|
||||
|
||||
let tab_size = Self::tab_size(&buffer, cx);
|
||||
// Important: obtain the snapshot BEFORE creating the subscription.
|
||||
// snapshot() may call sync() which publishes edits. If we subscribe first,
|
||||
// those edits would be captured but the InlayMap would already be at the
|
||||
// post-edit state, causing a desync.
|
||||
let buffer_snapshot = buffer.read(cx).snapshot(cx);
|
||||
let buffer_subscription = buffer.update(cx, |buffer, _| buffer.subscribe());
|
||||
let crease_map = CreaseMap::new(&buffer_snapshot);
|
||||
let (inlay_map, snapshot) = InlayMap::new(buffer_snapshot);
|
||||
let (fold_map, snapshot) = FoldMap::new(snapshot);
|
||||
|
|
@ -4079,4 +4082,64 @@ pub mod tests {
|
|||
chunks,
|
||||
);
|
||||
}
|
||||
|
||||
/// Regression test: Creating a DisplayMap when the MultiBuffer has pending
|
||||
/// unsynced changes should not cause a desync between the subscription edits
|
||||
/// and the InlayMap's buffer state.
|
||||
///
|
||||
/// The bug occurred because:
|
||||
/// 1. DisplayMap::new created a subscription first
|
||||
/// 2. Then called snapshot() which synced and published edits
|
||||
/// 3. InlayMap was created with the post-sync snapshot
|
||||
/// 4. But the subscription captured the sync edits, leading to double-application
|
||||
#[gpui::test]
|
||||
fn test_display_map_subscription_ordering(cx: &mut gpui::App) {
|
||||
init_test(cx, &|_| {});
|
||||
|
||||
// Create a buffer with some initial text
|
||||
let buffer = cx.new(|cx| Buffer::local("initial", cx));
|
||||
let multibuffer = cx.new(|cx| MultiBuffer::singleton(buffer.clone(), cx));
|
||||
|
||||
// Edit the buffer. This sets buffer_changed_since_sync = true.
|
||||
// Importantly, do NOT call multibuffer.snapshot() yet.
|
||||
buffer.update(cx, |buffer, cx| {
|
||||
buffer.edit([(0..0, "prefix ")], None, cx);
|
||||
});
|
||||
|
||||
// Create the DisplayMap. In the buggy code, this would:
|
||||
// 1. Create subscription (empty)
|
||||
// 2. Call snapshot() which syncs and publishes edits E1
|
||||
// 3. Create InlayMap with post-E1 snapshot
|
||||
// 4. Subscription now has E1, but InlayMap is already at post-E1 state
|
||||
let map = cx.new(|cx| {
|
||||
DisplayMap::new(
|
||||
multibuffer.clone(),
|
||||
font("Helvetica"),
|
||||
px(14.0),
|
||||
None,
|
||||
1,
|
||||
1,
|
||||
FoldPlaceholder::test(),
|
||||
DiagnosticSeverity::Warning,
|
||||
cx,
|
||||
)
|
||||
});
|
||||
|
||||
// Verify initial state is correct
|
||||
let snapshot = map.update(cx, |map, cx| map.snapshot(cx));
|
||||
assert_eq!(snapshot.text(), "prefix initial");
|
||||
|
||||
// Make another edit
|
||||
buffer.update(cx, |buffer, cx| {
|
||||
buffer.edit([(7..7, "more ")], None, cx);
|
||||
});
|
||||
|
||||
// This would crash in the buggy code because:
|
||||
// - InlayMap expects edits from V1 to V2
|
||||
// - But subscription has E1 ∘ E2 (from V0 to V2)
|
||||
// - The calculation `buffer_edit.new.end + (cursor.end().0 - buffer_edit.old.end)`
|
||||
// would produce an offset exceeding the buffer length
|
||||
let snapshot = map.update(cx, |map, cx| map.snapshot(cx));
|
||||
assert_eq!(snapshot.text(), "prefix more initial");
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue