fix side-comments, add tabs or spaces functionality

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "nufmt"
-version = "0.1.3"
+version = "0.1.4"
 edition = "2021"
 authors = ["The NuShell Contributors"]
 license = "MIT"

README.md

@@ -110,6 +110,7 @@ Create a `nufmt.nuon` file in your project root:
 ```nuon
 {
     indent: 4
+    indent_char: "space"
     line_length: 80
     margin: 1
     exclude: ["vendor/**", "target/**"]
@@ -120,7 +121,8 @@ Configuration options:
 
 | Option | Type | Default | Description |
 |--------|------|---------|-------------|
-| `indent` | int | 4 | Number of spaces per indentation level |
+| `indent` | int | 4 | Visual indentation width per level (tab width when `indent_char = "tab"`) |
+| `indent_char` | string | `"space"` | Indentation character: `"space"` or `"tab"` |
 | `line_length` | int | 80 | Maximum line length (advisory) |
 | `margin` | int | 1 | Number of blank lines between top-level items |
 | `exclude` | list\<string\> | [] | Glob patterns for files to exclude |

src/config.rs

@@ -5,11 +5,24 @@ use std::convert::TryFrom;
 use crate::config_error::ConfigError;
 use nu_protocol::Value;
 
+/// Character used for indentation.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum IndentChar {
+    Space,
+    Tab,
+}
+
 /// Configuration options for the formatter
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Config {
-    /// Number of spaces per indentation level (default: 4).
+    /// Visual indentation width per level (default: 4).
+    ///
+    /// With `indent_char = "space"`, this is the number of spaces written.
+    /// With `indent_char = "tab"`, this is the virtual tab width used in
+    /// layout calculations while writing one tab per indentation level.
     pub indent: usize,
+    /// Character used for each indentation unit (`space` or `tab`).
+    pub indent_char: IndentChar,
     /// Maximum line length before wrapping (default: 80).
     pub line_length: usize,
     /// Number of blank lines to insert between top-level definitions (default: 1).
@@ -28,6 +41,7 @@ impl Default for Config {
     fn default() -> Self {
         Self {
             indent: 4,
+            indent_char: IndentChar::Space,
             line_length: 80,
             margin: 1,
             margin_is_explicit: false,
@@ -44,6 +58,7 @@ impl Config {
     pub fn new(tab_spaces: usize, max_width: usize, margin: usize) -> Self {
         Self {
             indent: tab_spaces,
+            indent_char: IndentChar::Space,
             line_length: max_width,
             margin,
             margin_is_explicit: true,
@@ -69,6 +84,7 @@ impl TryFrom<Value> for Config {
         for (key, value) in record.iter() {
             match key.as_str() {
                 "indent" => config.indent = parse_positive_int(key, value)?,
+                "indent_char" => config.indent_char = parse_indent_char(value)?,
                 "line_length" => config.line_length = parse_positive_int(key, value)?,
                 "margin" => {
                     config.margin = parse_non_negative_int(key, value)?;
@@ -122,6 +138,27 @@ fn parse_non_negative_int(key: &str, value: &Value) -> Result<usize, ConfigError
     parse_int_at_least(key, value, 0, "a non-negative number")
 }
 
+/// Parse a value as the indentation character.
+fn parse_indent_char(value: &Value) -> Result<IndentChar, ConfigError> {
+    let Value::String { val, .. } = value else {
+        return Err(ConfigError::InvalidOptionType(
+            "indent_char".to_string(),
+            value.get_type().to_string(),
+            "string",
+        ));
+    };
+
+    match val.as_str() {
+        "space" => Ok(IndentChar::Space),
+        "tab" => Ok(IndentChar::Tab),
+        _ => Err(ConfigError::InvalidOptionValue(
+            "indent_char".to_string(),
+            val.clone(),
+            "space or tab",
+        )),
+    }
+}
+
 /// Parse a `Value` as a `list<string>` and return the strings.
 fn parse_string_list(value: &Value) -> Result<Vec<String>, ConfigError> {
     let Value::List { vals, .. } = value else {

src/formatting/calls.rs

@@ -164,7 +164,7 @@ impl<'a> Formatter<'a> {
             return false;
         }
 
-        source_span.len() > self.config.line_length
+        source_span.len() + (self.config.indent * self.indent_level) > self.config.line_length
     }
 
     /// Format a long regular call as:
@@ -1111,7 +1111,7 @@ impl<'a> Formatter<'a> {
                 .sum::<usize>()
             + sig.required_positional.len().saturating_sub(1) * 2;
 
-        inline_len <= self.config.line_length
+        inline_len + (self.config.indent * self.indent_level) <= self.config.line_length
     }
 
     // ─────────────────────────────────────────────────────────────────────────

src/formatting/collections.rs

@@ -15,23 +15,26 @@ impl<'a> Formatter<'a> {
     // ─────────────────────────────────────────────────────────────────────────
 
     /// Format a list expression, choosing inline or multiline layout.
-    pub(super) fn format_list(&mut self, items: &[ListItem]) {
+    pub(super) fn format_list(&mut self, items: &[ListItem], span: Span) {
         if items.is_empty() {
             self.write("[]");
             return;
         }
 
         let uses_commas = self.list_uses_commas(items);
         let source_has_newline = self.list_has_source_newline(items);
+        let has_comments_in_list = self.has_comments_in_span(span.start, span.end);
 
         let all_simple = items.iter().all(|item| match item {
             ListItem::Item(expr) | ListItem::Spread(_, expr) => self.is_simple_expr(expr),
         });
 
         let inline_single_item = items.len() == 1 && all_simple;
         let should_preserve_multiline = source_has_newline && items.len() > 1;
-        let should_inline =
-            inline_single_item || (all_simple && items.len() <= 5 && !should_preserve_multiline);
+        let should_inline = !has_comments_in_list
+            && (inline_single_item
+                || (all_simple && items.len() <= 5 && !should_preserve_multiline))
+            && self.inline_list_fits_on_line(items, uses_commas);
 
         if should_inline {
             self.write("[");
@@ -48,17 +51,25 @@ impl<'a> Formatter<'a> {
             self.write("]");
         } else {
             self.write("[");
+            let first_item_start = self.list_item_bounds(&items[0]).0;
+            self.write_inline_comment_bounded(span.start.saturating_add(1), Some(first_item_start));
             self.newline();
             self.indent_level += 1;
+            let closing_bracket_pos = span.end.saturating_sub(1);
 
             let mut idx = 0;
             while idx < items.len() {
+                let (item_start, _) = self.list_item_bounds(&items[idx]);
+                self.write_comments_before(item_start);
                 self.write_indent();
 
                 if idx + 1 < items.len() {
                     let current = &items[idx];
                     let next = &items[idx + 1];
                     if self.should_pair_flag_value_items(current, next)
+                        && !self.list_item_has_inline_comment(current, Some(closing_bracket_pos))
+                        && !self.list_item_has_inline_comment(next, Some(closing_bracket_pos))
+                        && !self.list_items_have_comments_between(current, next)
                         && self.paired_list_items_fit_on_line(current, next, uses_commas)
                     {
                         self.format_list_item(current);
@@ -68,17 +79,24 @@ impl<'a> Formatter<'a> {
                             self.write(" ");
                         }
                         self.format_list_item(next);
+                        let (_, next_end) = self.list_item_bounds(next);
+                        self.write_inline_comment_bounded(next_end, Some(closing_bracket_pos));
+                        self.last_pos = self.last_pos.max(next_end);
                         self.newline();
                         idx += 2;
                         continue;
                     }
                 }
 
                 self.format_list_item(&items[idx]);
+                let (_, item_end) = self.list_item_bounds(&items[idx]);
+                self.write_inline_comment_bounded(item_end, Some(closing_bracket_pos));
+                self.last_pos = self.last_pos.max(item_end);
                 self.newline();
                 idx += 1;
             }
 
+            self.write_comments_before(closing_bracket_pos);
             self.indent_level -= 1;
             self.write_indent();
             self.write("]");
@@ -141,6 +159,25 @@ impl<'a> Formatter<'a> {
         self.list_item_is_flag_string(current) && !self.list_item_is_flag_string(next)
     }
 
+    fn list_items_have_comments_between(&self, current: &ListItem, next: &ListItem) -> bool {
+        let (_, current_end) = self.list_item_bounds(current);
+        let (next_start, _) = self.list_item_bounds(next);
+        self.has_comments_in_span(current_end, next_start)
+    }
+
+    fn list_item_has_inline_comment(&self, item: &ListItem, upper: Option<usize>) -> bool {
+        let (_, item_end) = self.list_item_bounds(item);
+        let line_end = self.source[item_end..]
+            .iter()
+            .position(|&b| b == b'\n')
+            .map_or(self.source.len(), |p| item_end + p);
+        let effective_end = upper.map_or(line_end, |u| u.min(line_end));
+
+        self.comments.iter().enumerate().any(|(i, (span, _))| {
+            !self.written_comments[i] && span.start >= item_end && span.start < effective_end
+        })
+    }
+
     fn list_item_is_flag_string(&self, item: &ListItem) -> bool {
         let expr = match item {
             ListItem::Item(expr) => expr,
@@ -161,6 +198,20 @@ impl<'a> Formatter<'a> {
         inner.starts_with(b"-")
     }
 
+    fn inline_list_fits_on_line(&self, items: &[ListItem], uses_commas: bool) -> bool {
+        let mut total_len = 2; // for [ and ]
+        for (i, item) in items.iter().enumerate() {
+            let item_len = self.probe_format(|p| p.format_list_item(item)).len();
+            total_len += item_len;
+            if i > 0 {
+                total_len += if uses_commas { 2 } else { 1 }; // separator
+            }
+        }
+
+        let indent_len = self.config.indent * self.indent_level;
+        indent_len + total_len <= self.config.line_length
+    }
+
     fn paired_list_items_fit_on_line(
         &self,
         current: &ListItem,

src/formatting/expressions.rs

@@ -92,7 +92,7 @@ impl<'a> Formatter<'a> {
                 self.format_subexpression(*block_id, expr.span);
             }
 
-            Expr::List(items) => self.format_list(items),
+            Expr::List(items) => self.format_list(items, expr.span),
             Expr::Record(items) => self.format_record(items, expr.span),
             Expr::Table(table) => self.format_table(&table.columns, &table.rows),
 

src/formatting/mod.rs

@@ -21,7 +21,7 @@ mod expressions;
 mod garbage;
 mod repair;
 
-use crate::config::Config;
+use crate::config::{Config, IndentChar};
 use crate::format_error::FormatError;
 use log::{debug, trace};
 use nu_parser::parse;
@@ -133,8 +133,21 @@ impl<'a> Formatter<'a> {
     /// Write indentation if at the start of a line.
     pub(crate) fn write_indent(&mut self) {
         if self.at_line_start {
-            let indent = " ".repeat(self.config.indent * self.indent_level);
-            self.output.extend(indent.as_bytes());
+            match self.config.indent_char {
+                IndentChar::Space => {
+                    let num_spaces = self.config.indent * self.indent_level;
+                    self.output.reserve(num_spaces);
+                    for _ in 0..num_spaces {
+                        self.output.push(b' ');
+                    }
+                }
+                IndentChar::Tab => {
+                    self.output.reserve(self.indent_level);
+                    for _ in 0..self.indent_level {
+                        self.output.push(b'\t');
+                    }
+                }
+            }
             self.at_line_start = false;
         }
     }

src/main.rs

@@ -471,14 +471,20 @@ mod tests {
 
     #[rstest]
     #[case(r#"{line_length: 120, exclude: ["a*nu", "b*.nu"]}"#)]
+    #[case(r#"{indent_char: "tab", line_length: 120}"#)]
     fn read_valid_config(#[case] config_content: &str) {
         let dir = tempdir().unwrap();
         let config_file = dir.path().join("nufmt.nuon");
         fs::write(&config_file, config_content).unwrap();
 
         let config = read_config(&config_file).expect("Config should be valid");
         assert_eq!(config.line_length, 120_usize);
-        assert_eq!(config.excludes.len(), 2_usize);
+        if config_content.contains("exclude") {
+            assert_eq!(config.excludes.len(), 2_usize);
+        }
+        if config_content.contains(r#"indent_char: "tab""#) {
+            assert_eq!(config.indent_char, nu_formatter::config::IndentChar::Tab);
+        }
     }
 
     #[rstest]
@@ -488,6 +494,8 @@ mod tests {
     #[case(r#"{line_length: "120"}"#)]
     #[case(r#"{exclude: "a*nu"}"#)]
     #[case(r#"{exclude: ["a*nu", 1]}"#)]
+    #[case(r#"{indent_char: 1}"#)]
+    #[case(r#"{indent_char: "tabs"}"#)]
     fn read_invalid_config(#[case] config_content: &str) {
         let dir = tempdir().unwrap();
         let config_file = dir.path().join("nufmt.nuon");

tests/fixtures/config/indentation_respects_line_length.nuon

@@ -0,0 +1,5 @@
+{
+indent_char: "space"
+indent: 20
+line_length: 40
+}

tests/fixtures/config/tab_indentation_via_config_issue196.nuon

@@ -0,0 +1,6 @@
+{
+    indent: 4
+    indent_char: "tab"
+    line_length: 80
+    margin: 1
+}

tests/fixtures/expected/indentation_respects_line_length.nu

@@ -0,0 +1,13 @@
+def test [] {
+                    {
+                                        (echo
+                                                            abv
+                                                            def
+                                                            fef
+                                                            faei
+                                                            feal
+                                                            afea
+                                                            abcdefgh
+                                        )
+                    }
+}

tests/fixtures/expected/side_comments_in_multiline_lists_preserved_issue195.nu

@@ -0,0 +1,19 @@
+let flags = [
+    # opener-side comment should survive
+    "--name" # keep this side comment
+    "alice" # and this one too
+    # keep this standalone comment
+    "--age"
+    42 # keep numeric side comment
+]
+
+let single = [
+    1 # keep single-item side comment
+]
+
+const SIT = [ # SUPPORTED_INSTALLATION_TYPES
+    'cargo-git'
+    'crate'
+    'git-module'
+    'git-script'
+]

tests/fixtures/expected/tab_indentation_via_config_issue196.nu

@@ -0,0 +1,7 @@
+def greet [name] {
+	if $name != "" {
+		print $"Hello ($name)"
+	} else {
+		print "Hello"
+	}
+}

tests/fixtures/input/indentation_respects_line_length.nu

@@ -0,0 +1,5 @@
+def test [] {
+{
+echo abv def fef faei feal afea abcdefgh
+}
+}