Merge pull request #1457 from zed-industries/fix-tab-size-hang

Antonio Scandurra created

Prevent Zed from hanging when changing tab size

Change summary

crates/editor/src/display_map.rs          | 21 ++++
crates/editor/src/display_map/tab_map.rs  | 94 ++++++++++++++----------
crates/editor/src/display_map/wrap_map.rs |  4 
3 files changed, 73 insertions(+), 46 deletions(-)

Detailed changes

crates/editor/src/display_map.rs 🔗

@@ -589,7 +589,7 @@ pub mod tests {
             .unwrap_or(10);
 
         let font_cache = cx.font_cache().clone();
-        let tab_size = rng.gen_range(1..=4);
+        let mut tab_size = rng.gen_range(1..=4);
         let buffer_start_excerpt_header_height = rng.gen_range(1..=5);
         let excerpt_header_height = rng.gen_range(1..=5);
         let family_id = font_cache.load_family(&["Helvetica"]).unwrap();
@@ -607,7 +607,11 @@ pub mod tests {
         log::info!("tab size: {}", tab_size);
         log::info!("wrap width: {:?}", wrap_width);
 
-        cx.update(|cx| cx.set_global(Settings::test(cx)));
+        cx.update(|cx| {
+            let mut settings = Settings::test(cx);
+            settings.editor_overrides.tab_size = NonZeroU32::new(tab_size);
+            cx.set_global(settings)
+        });
 
         let buffer = cx.update(|cx| {
             if rng.gen() {
@@ -653,7 +657,18 @@ pub mod tests {
                     log::info!("setting wrap width to {:?}", wrap_width);
                     map.update(cx, |map, cx| map.set_wrap_width(wrap_width, cx));
                 }
-                20..=44 => {
+                20..=29 => {
+                    let mut tab_sizes = vec![1, 2, 3, 4];
+                    tab_sizes.remove((tab_size - 1) as usize);
+                    tab_size = *tab_sizes.choose(&mut rng).unwrap();
+                    log::info!("setting tab size to {:?}", tab_size);
+                    cx.update(|cx| {
+                        let mut settings = Settings::test(cx);
+                        settings.editor_overrides.tab_size = NonZeroU32::new(tab_size);
+                        cx.set_global(settings)
+                    });
+                }
+                30..=44 => {
                     map.update(cx, |map, cx| {
                         if rng.gen() || blocks.is_empty() {
                             let buffer = map.snapshot(cx).buffer_snapshot;

crates/editor/src/display_map/tab_map.rs 🔗

@@ -16,6 +16,7 @@ impl TabMap {
         let snapshot = TabSnapshot {
             fold_snapshot: input,
             tab_size,
+            version: 0,
         };
         (Self(Mutex::new(snapshot.clone())), snapshot)
     }
@@ -27,56 +28,70 @@ impl TabMap {
         tab_size: NonZeroU32,
     ) -> (TabSnapshot, Vec<TabEdit>) {
         let mut old_snapshot = self.0.lock();
-        let max_offset = old_snapshot.fold_snapshot.len();
-        let new_snapshot = TabSnapshot {
+        let mut new_snapshot = TabSnapshot {
             fold_snapshot,
             tab_size,
+            version: old_snapshot.version,
         };
 
+        if old_snapshot.fold_snapshot.version != new_snapshot.fold_snapshot.version {
+            new_snapshot.version += 1;
+        }
+
+        let old_max_offset = old_snapshot.fold_snapshot.len();
         let mut tab_edits = Vec::with_capacity(fold_edits.len());
-        for fold_edit in &mut fold_edits {
-            let mut delta = 0;
-            for chunk in
-                old_snapshot
-                    .fold_snapshot
-                    .chunks(fold_edit.old.end..max_offset, false, None)
-            {
-                let patterns: &[_] = &['\t', '\n'];
-                if let Some(ix) = chunk.text.find(patterns) {
-                    if &chunk.text[ix..ix + 1] == "\t" {
-                        fold_edit.old.end.0 += delta + ix + 1;
-                        fold_edit.new.end.0 += delta + ix + 1;
+
+        if old_snapshot.tab_size == new_snapshot.tab_size {
+            for fold_edit in &mut fold_edits {
+                let mut delta = 0;
+                for chunk in old_snapshot.fold_snapshot.chunks(
+                    fold_edit.old.end..old_max_offset,
+                    false,
+                    None,
+                ) {
+                    let patterns: &[_] = &['\t', '\n'];
+                    if let Some(ix) = chunk.text.find(patterns) {
+                        if &chunk.text[ix..ix + 1] == "\t" {
+                            fold_edit.old.end.0 += delta + ix + 1;
+                            fold_edit.new.end.0 += delta + ix + 1;
+                        }
+
+                        break;
                     }
 
-                    break;
+                    delta += chunk.text.len();
                 }
-
-                delta += chunk.text.len();
             }
-        }
 
-        let mut ix = 1;
-        while ix < fold_edits.len() {
-            let (prev_edits, next_edits) = fold_edits.split_at_mut(ix);
-            let prev_edit = prev_edits.last_mut().unwrap();
-            let edit = &next_edits[0];
-            if prev_edit.old.end >= edit.old.start {
-                prev_edit.old.end = edit.old.end;
-                prev_edit.new.end = edit.new.end;
-                fold_edits.remove(ix);
-            } else {
-                ix += 1;
+            let mut ix = 1;
+            while ix < fold_edits.len() {
+                let (prev_edits, next_edits) = fold_edits.split_at_mut(ix);
+                let prev_edit = prev_edits.last_mut().unwrap();
+                let edit = &next_edits[0];
+                if prev_edit.old.end >= edit.old.start {
+                    prev_edit.old.end = edit.old.end;
+                    prev_edit.new.end = edit.new.end;
+                    fold_edits.remove(ix);
+                } else {
+                    ix += 1;
+                }
             }
-        }
 
-        for fold_edit in fold_edits {
-            let old_start = fold_edit.old.start.to_point(&old_snapshot.fold_snapshot);
-            let old_end = fold_edit.old.end.to_point(&old_snapshot.fold_snapshot);
-            let new_start = fold_edit.new.start.to_point(&new_snapshot.fold_snapshot);
-            let new_end = fold_edit.new.end.to_point(&new_snapshot.fold_snapshot);
+            for fold_edit in fold_edits {
+                let old_start = fold_edit.old.start.to_point(&old_snapshot.fold_snapshot);
+                let old_end = fold_edit.old.end.to_point(&old_snapshot.fold_snapshot);
+                let new_start = fold_edit.new.start.to_point(&new_snapshot.fold_snapshot);
+                let new_end = fold_edit.new.end.to_point(&new_snapshot.fold_snapshot);
+                tab_edits.push(TabEdit {
+                    old: old_snapshot.to_tab_point(old_start)..old_snapshot.to_tab_point(old_end),
+                    new: new_snapshot.to_tab_point(new_start)..new_snapshot.to_tab_point(new_end),
+                });
+            }
+        } else {
+            new_snapshot.version += 1;
             tab_edits.push(TabEdit {
-                old: old_snapshot.to_tab_point(old_start)..old_snapshot.to_tab_point(old_end),
-                new: new_snapshot.to_tab_point(new_start)..new_snapshot.to_tab_point(new_end),
+                old: TabPoint::zero()..old_snapshot.max_point(),
+                new: TabPoint::zero()..new_snapshot.max_point(),
             });
         }
 
@@ -89,6 +104,7 @@ impl TabMap {
 pub struct TabSnapshot {
     pub fold_snapshot: FoldSnapshot,
     pub tab_size: NonZeroU32,
+    pub version: usize,
 }
 
 impl TabSnapshot {
@@ -160,10 +176,6 @@ impl TabSnapshot {
         }
     }
 
-    pub fn version(&self) -> usize {
-        self.fold_snapshot.version
-    }
-
     pub fn chunks<'a>(
         &'a self,
         range: Range<TabPoint>,

crates/editor/src/display_map/wrap_map.rs 🔗

@@ -220,7 +220,7 @@ impl WrapMap {
         if !self.snapshot.interpolated {
             let mut to_remove_len = 0;
             for (tab_snapshot, _) in &self.pending_edits {
-                if tab_snapshot.version() <= self.snapshot.tab_snapshot.version() {
+                if tab_snapshot.version <= self.snapshot.tab_snapshot.version {
                     to_remove_len += 1;
                 } else {
                     break;
@@ -282,7 +282,7 @@ impl WrapMap {
         let was_interpolated = self.snapshot.interpolated;
         let mut to_remove_len = 0;
         for (tab_snapshot, edits) in &self.pending_edits {
-            if tab_snapshot.version() <= self.snapshot.tab_snapshot.version() {
+            if tab_snapshot.version <= self.snapshot.tab_snapshot.version {
                 to_remove_len += 1;
             } else {
                 let interpolated_edits = self.snapshot.interpolate(tab_snapshot.clone(), &edits);