Merge pull request #2059 from zed-industries/no-indent-adjustment-on-error

Max Brunsfeld created

Avoid adjusting indentation of lines inside of newly-created errors

Change summary

crates/language/src/buffer.rs                |  66 ++++++-
crates/language/src/buffer_tests.rs          | 195 ++++++++++++++++-----
crates/language/src/language.rs              |   2 
crates/language/src/syntax_map.rs            |  48 -----
crates/text/src/text.rs                      |  51 +++++
crates/zed/src/languages/rust/highlights.scm |   1 
crates/zed/src/languages/rust/indents.scm    |   1 
7 files changed, 257 insertions(+), 107 deletions(-)

Detailed changes

crates/language/src/buffer.rs 🔗

@@ -282,6 +282,7 @@ struct AutoindentRequestEntry {
 struct IndentSuggestion {
     basis_row: u32,
     delta: Ordering,
+    within_error: bool,
 }
 
 struct BufferChunkHighlights<'a> {
@@ -937,7 +938,7 @@ impl Buffer {
                 // Build a map containing the suggested indentation for each of the edited lines
                 // with respect to the state of the buffer before these edits. This map is keyed
                 // by the rows for these lines in the current state of the buffer.
-                let mut old_suggestions = BTreeMap::<u32, IndentSize>::default();
+                let mut old_suggestions = BTreeMap::<u32, (IndentSize, bool)>::default();
                 let old_edited_ranges =
                     contiguous_ranges(old_to_new_rows.keys().copied(), max_rows_between_yields);
                 let mut language_indent_sizes = language_indent_sizes_by_new_row.iter().peekable();
@@ -963,14 +964,17 @@ impl Buffer {
 
                             let suggested_indent = old_to_new_rows
                                 .get(&suggestion.basis_row)
-                                .and_then(|from_row| old_suggestions.get(from_row).copied())
+                                .and_then(|from_row| {
+                                    Some(old_suggestions.get(from_row).copied()?.0)
+                                })
                                 .unwrap_or_else(|| {
                                     request
                                         .before_edit
                                         .indent_size_for_line(suggestion.basis_row)
                                 })
                                 .with_delta(suggestion.delta, language_indent_size);
-                            old_suggestions.insert(new_row, suggested_indent);
+                            old_suggestions
+                                .insert(new_row, (suggested_indent, suggestion.within_error));
                         }
                     }
                     yield_now().await;
@@ -1016,12 +1020,13 @@ impl Buffer {
                                     snapshot.indent_size_for_line(suggestion.basis_row)
                                 })
                                 .with_delta(suggestion.delta, language_indent_size);
-                            if old_suggestions
-                                .get(&new_row)
-                                .map_or(true, |old_indentation| {
+                            if old_suggestions.get(&new_row).map_or(
+                                true,
+                                |(old_indentation, was_within_error)| {
                                     suggested_indent != *old_indentation
-                                })
-                            {
+                                        && (!suggestion.within_error || *was_within_error)
+                                },
+                            ) {
                                 indent_sizes.insert(new_row, suggested_indent);
                             }
                         }
@@ -1661,6 +1666,16 @@ impl Buffer {
 
 #[cfg(any(test, feature = "test-support"))]
 impl Buffer {
+    pub fn edit_via_marked_text(
+        &mut self,
+        marked_string: &str,
+        autoindent_mode: Option<AutoindentMode>,
+        cx: &mut ModelContext<Self>,
+    ) {
+        let edits = self.edits_for_marked_text(marked_string);
+        self.edit(edits, autoindent_mode, cx);
+    }
+
     pub fn set_group_interval(&mut self, group_interval: Duration) {
         self.text.set_group_interval(group_interval);
     }
@@ -1779,7 +1794,7 @@ impl BufferSnapshot {
         let start = Point::new(prev_non_blank_row.unwrap_or(row_range.start), 0);
         let end = Point::new(row_range.end, 0);
         let range = (start..end).to_offset(&self.text);
-        let mut matches = self.syntax.matches(range, &self.text, |grammar| {
+        let mut matches = self.syntax.matches(range.clone(), &self.text, |grammar| {
             Some(&grammar.indents_config.as_ref()?.query)
         });
         let indent_configs = matches
@@ -1825,6 +1840,30 @@ impl BufferSnapshot {
             }
         }
 
+        let mut error_ranges = Vec::<Range<Point>>::new();
+        let mut matches = self.syntax.matches(range.clone(), &self.text, |grammar| {
+            Some(&grammar.error_query)
+        });
+        while let Some(mat) = matches.peek() {
+            let node = mat.captures[0].node;
+            let start = Point::from_ts_point(node.start_position());
+            let end = Point::from_ts_point(node.end_position());
+            let range = start..end;
+            let ix = match error_ranges.binary_search_by_key(&range.start, |r| r.start) {
+                Ok(ix) | Err(ix) => ix,
+            };
+            let mut end_ix = ix;
+            while let Some(existing_range) = error_ranges.get(end_ix) {
+                if existing_range.end < end {
+                    end_ix += 1;
+                } else {
+                    break;
+                }
+            }
+            error_ranges.splice(ix..end_ix, [range]);
+            matches.advance();
+        }
+
         outdent_positions.sort();
         for outdent_position in outdent_positions {
             // find the innermost indent range containing this outdent_position
@@ -1902,33 +1941,42 @@ impl BufferSnapshot {
                 }
             }
 
+            let within_error = error_ranges
+                .iter()
+                .any(|e| e.start.row < row && e.end > row_start);
+
             let suggestion = if outdent_to_row == prev_row
                 || (outdent_from_prev_row && indent_from_prev_row)
             {
                 Some(IndentSuggestion {
                     basis_row: prev_row,
                     delta: Ordering::Equal,
+                    within_error,
                 })
             } else if indent_from_prev_row {
                 Some(IndentSuggestion {
                     basis_row: prev_row,
                     delta: Ordering::Greater,
+                    within_error,
                 })
             } else if outdent_to_row < prev_row {
                 Some(IndentSuggestion {
                     basis_row: outdent_to_row,
                     delta: Ordering::Equal,
+                    within_error,
                 })
             } else if outdent_from_prev_row {
                 Some(IndentSuggestion {
                     basis_row: prev_row,
                     delta: Ordering::Less,
+                    within_error,
                 })
             } else if config.auto_indent_using_last_non_empty_line || !self.is_line_blank(prev_row)
             {
                 Some(IndentSuggestion {
                     basis_row: prev_row,
                     delta: Ordering::Equal,
+                    within_error,
                 })
             } else {
                 None

crates/language/src/buffer_tests.rs 🔗

@@ -800,23 +800,29 @@ fn test_autoindent_does_not_adjust_lines_with_unchanged_suggestion(cx: &mut Muta
     cx.set_global(settings);
 
     cx.add_model(|cx| {
-        let text = "
+        let mut buffer = Buffer::new(
+            0,
+            "
             fn a() {
             c;
             d;
             }
-        "
-        .unindent();
-
-        let mut buffer = Buffer::new(0, text, cx).with_language(Arc::new(rust_lang()), cx);
+            "
+            .unindent(),
+            cx,
+        )
+        .with_language(Arc::new(rust_lang()), cx);
 
         // Lines 2 and 3 don't match the indentation suggestion. When editing these lines,
         // their indentation is not adjusted.
-        buffer.edit(
-            [
-                (empty(Point::new(1, 1)), "()"),
-                (empty(Point::new(2, 1)), "()"),
-            ],
+        buffer.edit_via_marked_text(
+            &"
+            fn a() {
+            c«()»;
+            d«()»;
+            }
+            "
+            .unindent(),
             Some(AutoindentMode::EachLine),
             cx,
         );
@@ -833,14 +839,22 @@ fn test_autoindent_does_not_adjust_lines_with_unchanged_suggestion(cx: &mut Muta
 
         // When appending new content after these lines, the indentation is based on the
         // preceding lines' actual indentation.
-        buffer.edit(
-            [
-                (empty(Point::new(1, 1)), "\n.f\n.g"),
-                (empty(Point::new(2, 1)), "\n.f\n.g"),
-            ],
+        buffer.edit_via_marked_text(
+            &"
+            fn a() {
+            c«
+            .f
+            .g()»;
+            d«
+            .f
+            .g()»;
+            }
+            "
+            .unindent(),
             Some(AutoindentMode::EachLine),
             cx,
         );
+
         assert_eq!(
             buffer.text(),
             "
@@ -859,20 +873,90 @@ fn test_autoindent_does_not_adjust_lines_with_unchanged_suggestion(cx: &mut Muta
     });
 
     cx.add_model(|cx| {
-        let text = "
+        let mut buffer = Buffer::new(
+            0,
+            "
+            fn a() {
+                b();
+                |
+            "
+            .replace("|", "") // marker to preserve trailing whitespace
+            .unindent(),
+            cx,
+        )
+        .with_language(Arc::new(rust_lang()), cx);
+
+        // Insert a closing brace. It is outdented.
+        buffer.edit_via_marked_text(
+            &"
+            fn a() {
+                b();
+                «}»
+            "
+            .unindent(),
+            Some(AutoindentMode::EachLine),
+            cx,
+        );
+        assert_eq!(
+            buffer.text(),
+            "
+            fn a() {
+                b();
+            }
+            "
+            .unindent()
+        );
+
+        // Manually edit the leading whitespace. The edit is preserved.
+        buffer.edit_via_marked_text(
+            &"
+            fn a() {
+                b();
+            «    »}
+            "
+            .unindent(),
+            Some(AutoindentMode::EachLine),
+            cx,
+        );
+        assert_eq!(
+            buffer.text(),
+            "
             fn a() {
-                {
-                    b()?
+                b();
                 }
-                Ok(())
+            "
+            .unindent()
+        );
+        buffer
+    });
+}
+
+#[gpui::test]
+fn test_autoindent_does_not_adjust_lines_within_newly_created_errors(cx: &mut MutableAppContext) {
+    let settings = Settings::test(cx);
+    cx.set_global(settings);
+
+    cx.add_model(|cx| {
+        let mut buffer = Buffer::new(
+            0,
+            "
+            fn a() {
+                i
             }
-        "
-        .unindent();
-        let mut buffer = Buffer::new(0, text, cx).with_language(Arc::new(rust_lang()), cx);
+            "
+            .unindent(),
+            cx,
+        )
+        .with_language(Arc::new(rust_lang()), cx);
 
-        // Delete a closing curly brace changes the suggested indent for the line.
-        buffer.edit(
-            [(Point::new(3, 4)..Point::new(3, 5), "")],
+        // Regression test: line does not get outdented due to syntax error
+        buffer.edit_via_marked_text(
+            &"
+            fn a() {
+                i«f let Some(x) = y»
+            }
+            "
+            .unindent(),
             Some(AutoindentMode::EachLine),
             cx,
         );
@@ -880,19 +964,19 @@ fn test_autoindent_does_not_adjust_lines_with_unchanged_suggestion(cx: &mut Muta
             buffer.text(),
             "
             fn a() {
-                {
-                    b()?
-                        |
-                Ok(())
+                if let Some(x) = y
             }
             "
-            .replace('|', "") // included in the string to preserve trailing whites
             .unindent()
         );
 
-        // Manually editing the leading whitespace
-        buffer.edit(
-            [(Point::new(3, 0)..Point::new(3, 12), "")],
+        buffer.edit_via_marked_text(
+            &"
+            fn a() {
+                if let Some(x) = y« {»
+            }
+            "
+            .unindent(),
             Some(AutoindentMode::EachLine),
             cx,
         );
@@ -900,14 +984,12 @@ fn test_autoindent_does_not_adjust_lines_with_unchanged_suggestion(cx: &mut Muta
             buffer.text(),
             "
             fn a() {
-                {
-                    b()?
-
-                Ok(())
+                if let Some(x) = y {
             }
             "
             .unindent()
         );
+
         buffer
     });
 }
@@ -916,27 +998,42 @@ fn test_autoindent_does_not_adjust_lines_with_unchanged_suggestion(cx: &mut Muta
 fn test_autoindent_adjusts_lines_when_only_text_changes(cx: &mut MutableAppContext) {
     cx.set_global(Settings::test(cx));
     cx.add_model(|cx| {
-        let text = "
+        let mut buffer = Buffer::new(
+            0,
+            "
             fn a() {}
-        "
-        .unindent();
-
-        let mut buffer = Buffer::new(0, text, cx).with_language(Arc::new(rust_lang()), cx);
+            "
+            .unindent(),
+            cx,
+        )
+        .with_language(Arc::new(rust_lang()), cx);
 
-        buffer.edit([(5..5, "\nb")], Some(AutoindentMode::EachLine), cx);
+        buffer.edit_via_marked_text(
+            &"
+            fn a(«
+            b») {}
+            "
+            .unindent(),
+            Some(AutoindentMode::EachLine),
+            cx,
+        );
         assert_eq!(
             buffer.text(),
             "
-                fn a(
-                    b) {}
+            fn a(
+                b) {}
             "
             .unindent()
         );
 
         // The indentation suggestion changed because `@end` node (a close paren)
         // is now at the beginning of the line.
-        buffer.edit(
-            [(Point::new(1, 4)..Point::new(1, 5), "")],
+        buffer.edit_via_marked_text(
+            &"
+            fn a(
+                ˇ) {}
+            "
+            .unindent(),
             Some(AutoindentMode::EachLine),
             cx,
         );
@@ -1894,7 +1991,3 @@ fn get_tree_sexp(buffer: &ModelHandle<Buffer>, cx: &gpui::TestAppContext) -> Str
         layers[0].node.to_sexp()
     })
 }
-
-fn empty(point: Point) -> Range<Point> {
-    point..point
-}

crates/language/src/language.rs 🔗

@@ -348,6 +348,7 @@ pub struct Language {
 pub struct Grammar {
     id: usize,
     pub(crate) ts_language: tree_sitter::Language,
+    pub(crate) error_query: Query,
     pub(crate) highlights_query: Option<Query>,
     pub(crate) brackets_config: Option<BracketConfig>,
     pub(crate) indents_config: Option<IndentConfig>,
@@ -684,6 +685,7 @@ impl Language {
                     indents_config: None,
                     injection_config: None,
                     override_config: None,
+                    error_query: Query::new(ts_language, "(ERROR) @error").unwrap(),
                     ts_language,
                     highlight_map: Default::default(),
                 })

crates/language/src/syntax_map.rs 🔗

@@ -2262,7 +2262,7 @@ mod tests {
         mutated_syntax_map.reparse(language.clone(), &buffer);
 
         for (i, marked_string) in steps.into_iter().enumerate() {
-            edit_buffer(&mut buffer, &marked_string.unindent());
+            buffer.edit_via_marked_text(&marked_string.unindent());
 
             // Reparse the syntax map
             mutated_syntax_map.interpolate(&buffer);
@@ -2452,52 +2452,6 @@ mod tests {
         assert_eq!(actual_ranges, expected_ranges);
     }
 
-    fn edit_buffer(buffer: &mut Buffer, marked_string: &str) {
-        let old_text = buffer.text();
-        let (new_text, mut ranges) = marked_text_ranges(marked_string, false);
-        if ranges.is_empty() {
-            ranges.push(0..new_text.len());
-        }
-
-        assert_eq!(
-            old_text[..ranges[0].start],
-            new_text[..ranges[0].start],
-            "invalid edit"
-        );
-
-        let mut delta = 0;
-        let mut edits = Vec::new();
-        let mut ranges = ranges.into_iter().peekable();
-
-        while let Some(inserted_range) = ranges.next() {
-            let new_start = inserted_range.start;
-            let old_start = (new_start as isize - delta) as usize;
-
-            let following_text = if let Some(next_range) = ranges.peek() {
-                &new_text[inserted_range.end..next_range.start]
-            } else {
-                &new_text[inserted_range.end..]
-            };
-
-            let inserted_len = inserted_range.len();
-            let deleted_len = old_text[old_start..]
-                .find(following_text)
-                .expect("invalid edit");
-
-            let old_range = old_start..old_start + deleted_len;
-            edits.push((old_range, new_text[inserted_range].to_string()));
-            delta += inserted_len as isize - deleted_len as isize;
-        }
-
-        assert_eq!(
-            old_text.len() as isize + delta,
-            new_text.len() as isize,
-            "invalid edit"
-        );
-
-        buffer.edit(edits);
-    }
-
     pub fn string_contains_sequence(text: &str, parts: &[&str]) -> bool {
         let mut last_part_end = 0;
         for part in parts {

crates/text/src/text.rs 🔗

@@ -1372,6 +1372,57 @@ impl Buffer {
 
 #[cfg(any(test, feature = "test-support"))]
 impl Buffer {
+    pub fn edit_via_marked_text(&mut self, marked_string: &str) {
+        let edits = self.edits_for_marked_text(marked_string);
+        self.edit(edits);
+    }
+
+    pub fn edits_for_marked_text(&self, marked_string: &str) -> Vec<(Range<usize>, String)> {
+        let old_text = self.text();
+        let (new_text, mut ranges) = util::test::marked_text_ranges(marked_string, false);
+        if ranges.is_empty() {
+            ranges.push(0..new_text.len());
+        }
+
+        assert_eq!(
+            old_text[..ranges[0].start],
+            new_text[..ranges[0].start],
+            "invalid edit"
+        );
+
+        let mut delta = 0;
+        let mut edits = Vec::new();
+        let mut ranges = ranges.into_iter().peekable();
+
+        while let Some(inserted_range) = ranges.next() {
+            let new_start = inserted_range.start;
+            let old_start = (new_start as isize - delta) as usize;
+
+            let following_text = if let Some(next_range) = ranges.peek() {
+                &new_text[inserted_range.end..next_range.start]
+            } else {
+                &new_text[inserted_range.end..]
+            };
+
+            let inserted_len = inserted_range.len();
+            let deleted_len = old_text[old_start..]
+                .find(following_text)
+                .expect("invalid edit");
+
+            let old_range = old_start..old_start + deleted_len;
+            edits.push((old_range, new_text[inserted_range].to_string()));
+            delta += inserted_len as isize - deleted_len as isize;
+        }
+
+        assert_eq!(
+            old_text.len() as isize + delta,
+            new_text.len() as isize,
+            "invalid edit"
+        );
+
+        edits
+    }
+
     pub fn check_invariants(&self) {
         // Ensure every fragment is ordered by locator in the fragment tree and corresponds
         // to an insertion fragment in the insertions tree.