Store alignment as a string in order to correctly align

to lines that contain intermixed tabs & other characters.

helix-core/src/indent.rs

@@ -240,23 +240,22 @@ fn get_first_in_line(mut node: Node, new_line_byte_pos: Option<usize>) -> Vec<bo
 /// This is usually constructed in one of 2 ways:
 /// - Successively add indent captures to get the (added) indent from a single line
 /// - Successively add the indent results for each line
-/// The string that this indentation defines depends on the indent style (spaces or tabs, etc.).
-/// However, it's width (as displayed) should equal the following number of spaces: max(0, align + (indent - outdent)*tab_width)
+/// The string that this indentation defines starts with the string contained in the align field (unless it is None), followed by:
+/// - max(0, indent - outdent) tabs, if tabs are used for indentation
+/// - max(0, indent - outdent)*indent_width spaces, if spaces are used for indentation
 #[derive(Default)]
 pub struct Indentation {
     indent: usize,
     outdent: usize,
-    /// The alignment given by (tabs, spaces). We need to store both in order to
-    /// ensure alignment for every tab width (in case tabs are used for indentation).
-    /// The indent/outdent resulting from lines below it will be added to this alignment.
-    /// The indentation of the line itself or lines that are higher up is ignored.
-    align: Option<(usize, usize)>,
+    /// The alignment, as a string containing only tabs & spaces. Storing this as a string instead of e.g.
+    /// the (visual) width ensures that the alignment is preserved even if the tab width changes.
+    align: Option<String>,
 }
 impl Indentation {
     /// Add some other [Indentation] to this.
     /// The added indent should be the total added indent from one line.
     /// Indent should always be added starting from the bottom (or equivalently, the innermost tree-sitter node).
-    fn add_line(&mut self, added: &Indentation) {
+    fn add_line(&mut self, added: Indentation) {
         // Align overrides the indent from outer scopes.
         if self.align.is_some() {
             return;
@@ -286,15 +285,15 @@ impl Indentation {
             }
         }
     }
-    fn as_string(&self, indent_style: &IndentStyle) -> String {
+    fn into_string(self, indent_style: &IndentStyle) -> String {
         let indent_level = if self.indent >= self.outdent {
             self.indent - self.outdent
         } else {
             log::warn!("Encountered more outdent than indent nodes while calculating indentation: {} outdent, {} indent", self.outdent, self.indent);
             0
         };
-        let mut indent_string = if let Some((align_tabs, align_spaces)) = self.align {
-            "\t".repeat(align_tabs) + " ".repeat(align_spaces).as_str()
+        let mut indent_string = if let Some(align) = self.align {
+            align
         } else {
             String::new()
         };
@@ -308,12 +307,12 @@ struct IndentCapture {
     capture_type: IndentCaptureType,
     scope: IndentScope,
 }
-#[derive(Clone, Copy)]
+#[derive(Clone)]
 enum IndentCaptureType {
     Indent,
     Outdent,
-    /// Alignment given as a pair (tabs, spaces)
-    Align((usize, usize)),
+    /// Alignment given as a string of whitespace
+    Align(String),
 }
 impl IndentCaptureType {
     fn default_scope(&self) -> IndentScope {
@@ -433,7 +432,7 @@ fn query_indents(
                 "indent" => IndentCaptureType::Indent,
                 "outdent" => IndentCaptureType::Outdent,
                 // The alignment will be updated to the correct value at the end, when the anchor is known.
-                "align" => IndentCaptureType::Align((0, 0)),
+                "align" => IndentCaptureType::Align(String::from("")),
                 "anchor" => {
                     if anchor.is_some() {
                         log::error!("Invalid indent query: Encountered more than one @anchor in the same match.")
@@ -505,16 +504,18 @@ fn query_indents(
                     }
                     Some(anchor) => anchor,
                 };
-                // We only count tabs that are part of the indentation for simplicity.
-                // This does not guarantee alignment in case there are non-leading tabs
-                // but this should never happen in reasonably formatted code.
-                let num_tabs = text
+                // Create a string of tabs & spaces that should have the same width
+                // as the string that precedes the anchor (independent of the tab width).
+                let align = text
                     .line(anchor.start_position().row)
+                    .byte_slice(0..anchor.start_position().column)
                     .chars()
-                    .take_while(|&c| c == '\t')
-                    .count();
-                let num_spaces = anchor.start_position().column - num_tabs;
-                capture.capture_type = IndentCaptureType::Align((num_tabs, num_spaces));
+                    .map(|c| match c {
+                        '\t' => '\t',
+                        _ => ' ',
+                    })
+                    .collect();
+                capture.capture_type = IndentCaptureType::Align(align);
             }
             indent_captures
                 .entry(node_id)
@@ -687,7 +688,7 @@ pub fn treesitter_indent_for_pos(
             (query_result, deepest_preceding)
         })
     };
-    let indent_captures = query_result.indent_captures;
+    let mut indent_captures = query_result.indent_captures;
     let extend_captures = query_result.extend_captures;
 
     // Check for extend captures, potentially changing the node that the indent calculation starts with
@@ -713,8 +714,10 @@ pub fn treesitter_indent_for_pos(
         // This can safely be unwrapped because `first_in_line` contains
         // one entry for each ancestor of the node (which is what we iterate over)
         let is_first = *first_in_line.last().unwrap();
-        // Apply all indent definitions for this node
-        if let Some(definitions) = indent_captures.get(&node.id()) {
+        // Apply all indent definitions for this node.
+        // Since we only iterate over each node once, we can remove the
+        // corresponding captures from the HashMap to avoid cloning them.
+        if let Some(definitions) = indent_captures.remove(&node.id()) {
             for definition in definitions {
                 match definition.scope {
                     IndentScope::All => {
@@ -737,12 +740,12 @@ pub fn treesitter_indent_for_pos(
             if node_line != parent_line {
                 if node_line < line + (new_line as usize) {
                     // Don't add indent for the line below the line of the query
-                    result.add_line(&indent_for_line_below);
+                    result.add_line(indent_for_line_below);
                 }
                 if node_line == parent_line + 1 {
                     indent_for_line_below = indent_for_line;
                 } else {
-                    result.add_line(&indent_for_line);
+                    result.add_line(indent_for_line);
                     indent_for_line_below = Indentation::default();
                 }
                 indent_for_line = Indentation::default();
@@ -756,13 +759,13 @@ pub fn treesitter_indent_for_pos(
             if (node.start_position().row < line)
                 || (new_line && node.start_position().row == line && node.start_byte() < byte_pos)
             {
-                result.add_line(&indent_for_line_below);
+                result.add_line(indent_for_line_below);
             }
-            result.add_line(&indent_for_line);
+            result.add_line(indent_for_line);
             break;
         }
     }
-    Some(result.as_string(indent_style))
+    Some(result.into_string(indent_style))
 }
 
 /// Returns the indentation for a new line.