snippets: Fix <tab> not working when at end of snippet (#11740)

Thorsten Ball created 2 years ago

This fixes #10185 by not keeping snippet state around when already at
the end of the snippet and the tabstop is empty (i.e. it's not a
selection) and we're already on it.

The reason for the fix is outlined in the comments of #10185 but to
repeat:

1. `gopls` sends completions with type "snippet" even when suggesting
single word completions that don't contain tabstops
2. We use a default behavior and add an "end tabstop" by default so that
the cursor jumps to the end of the snippet when accepting it.
3. We'd then push the state of the snippet on the stack which is where
it would stay, with the cursor already at the end and the user unable to
get rid of the tabstop state.

This fixes the issue by not pushing snippet state when the tabstop we
accepted is the "end tabstop".

Release Notes:

- Fixed completions inside snippets breaking the jump-to-next-tabstop
behaviour when using Go/`gopls`
([#10185](https://github.com/zed-industries/zed/issues/10185)).

Demo:



https://github.com/zed-industries/zed/assets/1185253/35384e5e-45c6-46ab-870d-b48e56d8786b

Change summary

crates/editor/src/editor.rs | 33 +++++++++++++++++++++++++++------
1 file changed, 27 insertions(+), 6 deletions(-)

Detailed changes

crates/editor/src/editor.rs

@@ -4636,6 +4636,11 @@ impl Editor {
         snippet: Snippet,
         cx: &mut ViewContext<Self>,
     ) -> Result<()> {
+        struct Tabstop<T> {
+            is_end_tabstop: bool,
+            ranges: Vec<Range<T>>,
+        }
+
         let tabstops = self.buffer.update(cx, |buffer, cx| {
             let snippet_text: Arc<str> = snippet.text.clone().into();
             buffer.edit(
@@ -4653,6 +4658,9 @@ impl Editor {
                 .tabstops
                 .iter()
                 .map(|tabstop| {
+                    let is_end_tabstop = tabstop.first().map_or(false, |tabstop| {
+                        tabstop.is_empty() && tabstop.start == snippet.text.len() as isize
+                    });
                     let mut tabstop_ranges = tabstop
                         .iter()
                         .flat_map(|tabstop_range| {
@@ -4672,20 +4680,33 @@ impl Editor {
                         })
                         .collect::<Vec<_>>();
                     tabstop_ranges.sort_unstable_by(|a, b| a.start.cmp(&b.start, snapshot));
-                    tabstop_ranges
+
+                    Tabstop {
+                        is_end_tabstop,
+                        ranges: tabstop_ranges,
+                    }
                 })
                 .collect::<Vec<_>>()
         });
 
         if let Some(tabstop) = tabstops.first() {
             self.change_selections(Some(Autoscroll::fit()), cx, |s| {
-                s.select_ranges(tabstop.iter().cloned());
-            });
-            self.snippet_stack.push(SnippetState {
-                active_index: 0,
-                ranges: tabstops,
+                s.select_ranges(tabstop.ranges.iter().cloned());
             });
 
+            // If we're already at the last tabstop and it's at the end of the snippet,
+            // we're done, we don't need to keep the state around.
+            if !tabstop.is_end_tabstop {
+                let ranges = tabstops
+                    .into_iter()
+                    .map(|tabstop| tabstop.ranges)
+                    .collect::<Vec<_>>();
+                self.snippet_stack.push(SnippetState {
+                    active_index: 0,
+                    ranges,
+                });
+            }
+
             // Check whether the just-entered snippet ends with an auto-closable bracket.
             if self.autoclose_regions.is_empty() {
                 let snapshot = self.buffer.read(cx).snapshot(cx);