From 7d268160b4d09c1314eda9df3e284a05ae4b6d60 Mon Sep 17 00:00:00 2001 From: Darren Schroeder <343840+fdncred@users.noreply.github.com> Date: Sun, 26 Apr 2026 19:10:31 -0500 Subject: [PATCH] feat: add support for alias command formatting and preserve comment spacing --- src/formatting/calls.rs | 85 ++++++++++++++++++ src/formatting/collections.rs | 15 +++- src/formatting/comments.rs | 89 ++++++++++++++++--- src/formatting/mod.rs | 1 + ...eferences_do_not_duplicate_rhs_issue167.nu | 3 + ..._comments_and_blocks_preserved_issue165.nu | 10 +++ ...line_record_comments_preserved_issue168.nu | 5 ++ ...eferences_do_not_duplicate_rhs_issue167.nu | 3 + ..._comments_and_blocks_preserved_issue165.nu | 10 +++ ...line_record_comments_preserved_issue168.nu | 5 ++ tests/ground_truth.rs | 15 ++++ tests/main.rs | 3 +- 12 files changed, 228 insertions(+), 16 deletions(-) create mode 100644 tests/fixtures/expected/alias_references_do_not_duplicate_rhs_issue167.nu create mode 100644 tests/fixtures/expected/empty_lines_between_comments_and_blocks_preserved_issue165.nu create mode 100644 tests/fixtures/expected/multiline_record_comments_preserved_issue168.nu create mode 100644 tests/fixtures/input/alias_references_do_not_duplicate_rhs_issue167.nu create mode 100644 tests/fixtures/input/empty_lines_between_comments_and_blocks_preserved_issue165.nu create mode 100644 tests/fixtures/input/multiline_record_comments_preserved_issue168.nu diff --git a/src/formatting/calls.rs b/src/formatting/calls.rs index 75341fe..e103a07 100644 --- a/src/formatting/calls.rs +++ b/src/formatting/calls.rs @@ -19,6 +19,7 @@ pub(super) const BLOCK_COMMANDS: &[&str] = &["for", "while", "loop", "module"]; pub(super) const CONDITIONAL_COMMANDS: &[&str] = &["if", "try"]; pub(super) const DEF_COMMANDS: &[&str] = &["def", "def-env", "export def"]; pub(super) const EXTERN_COMMANDS: &[&str] = &["extern", "export extern"]; +pub(super) const ALIAS_COMMANDS: &[&str] = &["alias", "export alias"]; pub(super) const LET_COMMANDS: &[&str] = &["let", "let-env", "mut", "const", "export const"]; impl<'a> Formatter<'a> { @@ -47,6 +48,11 @@ impl<'a> Formatter<'a> { return; } + if matches!(cmd_type, CommandType::Alias) { + self.format_alias_call(call); + return; + } + if decl_name == "for" { self.format_for_call(call); return; @@ -270,12 +276,90 @@ impl<'a> Formatter<'a> { } } + /// Format alias definitions while preserving the literal right-hand side. + /// + /// The parser resolves alias references semantically, which can expand the + /// RHS if it is re-rendered from the AST. Preserve the original source text + /// after `=` to keep alias definitions idempotent. + pub(super) fn format_alias_call(&mut self, call: &nu_protocol::ast::Call) { + let positional: Vec<&Expression> = call + .arguments + .iter() + .filter_map(|arg| match arg { + Argument::Positional(expr) | Argument::Unknown(expr) => Some(expr), + _ => None, + }) + .collect(); + + let Some(name) = positional.first() else { + for arg in &call.arguments { + self.format_call_argument(arg, &CommandType::Alias); + } + return; + }; + + self.space(); + self.format_expression(name); + + let rhs_end = call + .arguments + .iter() + .filter_map(|arg| match arg { + Argument::Positional(expr) | Argument::Unknown(expr) | Argument::Spread(expr) => { + Some(expr.span.end) + } + Argument::Named(named) => named + .2 + .as_ref() + .map_or(Some(named.0.span.end), |value| Some(value.span.end)), + }) + .max(); + + let Some(rhs_end) = rhs_end else { + return; + }; + + if name.span.end >= rhs_end || rhs_end > self.source.len() { + self.format_regular_arguments(&call.arguments[1..]); + return; + } + + let between = &self.source[name.span.end..rhs_end]; + let Some(eq_offset) = between.iter().position(|byte| *byte == b'=') else { + self.format_regular_arguments(&call.arguments[1..]); + return; + }; + + let rhs_start = between[eq_offset + 1..] + .iter() + .position(|byte| !byte.is_ascii_whitespace()) + .map(|offset| name.span.end + eq_offset + 1 + offset); + + let Some(rhs_start) = rhs_start else { + self.write(" ="); + return; + }; + + // Keep the exact user-authored alias RHS to avoid semantic expansion + // when aliases reference other aliases. + self.write(" = "); + self.write_bytes(&self.source[rhs_start..rhs_end]); + } + + fn format_regular_arguments(&mut self, args: &[Argument]) { + for arg in args { + self.format_call_argument(arg, &CommandType::Regular); + } + } + /// Classify a command name into a [`CommandType`] for formatting purposes. pub(super) fn classify_command(name: &str) -> CommandType { if DEF_COMMANDS.contains(&name) { CommandType::Def } else if EXTERN_COMMANDS.contains(&name) { CommandType::Extern + } else if ALIAS_COMMANDS.contains(&name) { + CommandType::Alias } else if CONDITIONAL_COMMANDS.contains(&name) { CommandType::Conditional } else if LET_COMMANDS.contains(&name) { @@ -336,6 +420,7 @@ impl<'a> Formatter<'a> { match cmd_type { CommandType::Def => self.format_def_argument(positional), CommandType::Extern => self.format_extern_argument(positional), + CommandType::Alias => self.format_expression(positional), CommandType::Conditional => { self.conditional_context_depth += 1; self.format_block_or_expr(positional); diff --git a/src/formatting/collections.rs b/src/formatting/collections.rs index ec26962..1150c94 100644 --- a/src/formatting/collections.rs +++ b/src/formatting/collections.rs @@ -258,17 +258,30 @@ impl<'a> Formatter<'a> { self.write("{"); self.newline(); self.indent_level += 1; + + let closing_brace_pos = span.end.saturating_sub(1); for item in items { + let item_start = match item { + RecordItem::Pair(key, _) => key.span.start, + RecordItem::Spread(_, expr) => expr.span.start, + }; + self.write_comments_before(item_start); self.write_indent(); self.format_record_item(item, preserve_compact); + // Capture inline comments on the same line as the record item. let item_end = match item { RecordItem::Pair(_, v) => v.span.end, RecordItem::Spread(_, expr) => expr.span.end, }; - self.write_inline_comment_bounded(item_end, None); + self.write_inline_comment_bounded(item_end, Some(closing_brace_pos)); + // Advance beyond the current item so inter-item comments are + // searched from the correct source window. + self.last_pos = self.last_pos.max(item_end); self.newline(); } + + self.write_comments_before(closing_brace_pos); self.indent_level -= 1; self.write_indent(); self.write("}"); diff --git a/src/formatting/comments.rs b/src/formatting/comments.rs index 68c69a6..4da2272 100644 --- a/src/formatting/comments.rs +++ b/src/formatting/comments.rs @@ -60,6 +60,55 @@ pub(super) fn extract_comments(source: &[u8]) -> Vec<(Span, Vec)> { // ───────────────────────────────────────────────────────────────────────────── impl<'a> Formatter<'a> { + /// Return true when the source slice contains at least one blank line + /// (two newline boundaries with only whitespace between them). + fn source_has_blank_line(&self, start: usize, end: usize) -> bool { + if start >= end || end > self.source.len() { + return false; + } + + let mut previous_newline: Option = None; + for (offset, byte) in self.source[start..end].iter().enumerate() { + if *byte != b'\n' { + continue; + } + + if let Some(prev) = previous_newline { + let gap_start = start + prev + 1; + let gap_end = start + offset; + if self.source[gap_start..gap_end] + .iter() + .all(|b| b.is_ascii_whitespace()) + { + return true; + } + } + + previous_newline = Some(offset); + } + + false + } + + fn ensure_trailing_newlines(&mut self, min_newlines: usize) { + if self.output.is_empty() || min_newlines == 0 { + return; + } + + // Count contiguous trailing newlines so we can top up to the requested + // separation without over-emitting line breaks. + let existing = self + .output + .iter() + .rev() + .take_while(|&&byte| byte == b'\n') + .count(); + + for _ in existing..min_newlines { + self.newline(); + } + } + /// Emit all comments that fall between `last_pos` and `pos`, each on its /// own line with the current indentation. pub(super) fn write_comments_before(&mut self, pos: usize) { @@ -75,24 +124,29 @@ impl<'a> Formatter<'a> { comments_to_write.sort_by_key(|(_, start, _)| *start); + let Some((_, first_start, _)) = comments_to_write.first() else { + return; + }; + + let leading_newlines = if self.source_has_blank_line(self.last_pos, *first_start) { + 2 + } else { + 1 + }; + // Preserve spacing before a standalone comment group. + self.ensure_trailing_newlines(leading_newlines); + let mut prev_comment_end: Option = None; for (idx, start, content) in &comments_to_write { self.written_comments[*idx] = true; - // Preserve blank lines between comment groups by checking - // whether the original source has a blank line between the - // previous comment and this one. if let Some(prev_end) = prev_comment_end { - let between = &self.source[prev_end..*start]; - let newline_count = between.iter().filter(|&&b| b == b'\n').count(); - if newline_count >= 2 { - // There was at least one blank line in the source - // between the two comments — emit one now. - if !self.at_line_start { - self.newline(); - } - self.newline(); - } + let between_newlines = if self.source_has_blank_line(prev_end, *start) { + 2 + } else { + 1 + }; + self.ensure_trailing_newlines(between_newlines); } if !self.at_line_start { @@ -108,6 +162,15 @@ impl<'a> Formatter<'a> { prev_comment_end = Some(start + content.len()); } + + if let Some(last_comment_end) = prev_comment_end { + self.last_pos = last_comment_end; + if self.source_has_blank_line(last_comment_end, pos) { + // Preserve a blank separator when comments are followed by a + // spaced-apart statement group. + self.ensure_trailing_newlines(2); + } + } } /// Emit an inline comment (on the same line) that appears after `after_pos`, diff --git a/src/formatting/mod.rs b/src/formatting/mod.rs index 8701ada..a2722a7 100644 --- a/src/formatting/mod.rs +++ b/src/formatting/mod.rs @@ -80,6 +80,7 @@ pub(crate) struct Formatter<'a> { pub(crate) enum CommandType { Def, Extern, + Alias, Conditional, Let, Block, diff --git a/tests/fixtures/expected/alias_references_do_not_duplicate_rhs_issue167.nu b/tests/fixtures/expected/alias_references_do_not_duplicate_rhs_issue167.nu new file mode 100644 index 0000000..344fafe --- /dev/null +++ b/tests/fixtures/expected/alias_references_do_not_duplicate_rhs_issue167.nu @@ -0,0 +1,3 @@ +alias fdr = fd --follow --hidden --no-ignore +alias fdf = fdr --follow --hidden --no-ignore --fixed-strings +alias fdg = fdr --follow --hidden --no-ignore --glob \ No newline at end of file diff --git a/tests/fixtures/expected/empty_lines_between_comments_and_blocks_preserved_issue165.nu b/tests/fixtures/expected/empty_lines_between_comments_and_blocks_preserved_issue165.nu new file mode 100644 index 0000000..5b5df29 --- /dev/null +++ b/tests/fixtures/expected/empty_lines_between_comments_and_blocks_preserved_issue165.nu @@ -0,0 +1,10 @@ +#!/usr/bin/env nu + +let x = 1 + +# This is a nice comment. +# +# It's not associated with any particular block. It just describes things about +# the contents of this file, or the broader section of code. + +let y = 2 \ No newline at end of file diff --git a/tests/fixtures/expected/multiline_record_comments_preserved_issue168.nu b/tests/fixtures/expected/multiline_record_comments_preserved_issue168.nu new file mode 100644 index 0000000..f6e8ae4 --- /dev/null +++ b/tests/fixtures/expected/multiline_record_comments_preserved_issue168.nu @@ -0,0 +1,5 @@ +let a = { + my_key: "my_key" + # a comment + my_other_key: "my_other_key" +} \ No newline at end of file diff --git a/tests/fixtures/input/alias_references_do_not_duplicate_rhs_issue167.nu b/tests/fixtures/input/alias_references_do_not_duplicate_rhs_issue167.nu new file mode 100644 index 0000000..344fafe --- /dev/null +++ b/tests/fixtures/input/alias_references_do_not_duplicate_rhs_issue167.nu @@ -0,0 +1,3 @@ +alias fdr = fd --follow --hidden --no-ignore +alias fdf = fdr --follow --hidden --no-ignore --fixed-strings +alias fdg = fdr --follow --hidden --no-ignore --glob \ No newline at end of file diff --git a/tests/fixtures/input/empty_lines_between_comments_and_blocks_preserved_issue165.nu b/tests/fixtures/input/empty_lines_between_comments_and_blocks_preserved_issue165.nu new file mode 100644 index 0000000..5b5df29 --- /dev/null +++ b/tests/fixtures/input/empty_lines_between_comments_and_blocks_preserved_issue165.nu @@ -0,0 +1,10 @@ +#!/usr/bin/env nu + +let x = 1 + +# This is a nice comment. +# +# It's not associated with any particular block. It just describes things about +# the contents of this file, or the broader section of code. + +let y = 2 \ No newline at end of file diff --git a/tests/fixtures/input/multiline_record_comments_preserved_issue168.nu b/tests/fixtures/input/multiline_record_comments_preserved_issue168.nu new file mode 100644 index 0000000..257c321 --- /dev/null +++ b/tests/fixtures/input/multiline_record_comments_preserved_issue168.nu @@ -0,0 +1,5 @@ +let a = { + my_key: "my_key" + # a comment + my_other_key: "my_other_key" +} \ No newline at end of file diff --git a/tests/ground_truth.rs b/tests/ground_truth.rs index 3b4647a..7bf0247 100644 --- a/tests/ground_truth.rs +++ b/tests/ground_truth.rs @@ -610,4 +610,19 @@ fixture_tests!( ground_truth_parens_stripping_boolean_exprs_issue162, idempotency_parens_stripping_boolean_exprs_issue162 ), + ( + "empty_lines_between_comments_and_blocks_preserved_issue165", + ground_truth_empty_lines_between_comments_and_blocks_preserved_issue165, + idempotency_empty_lines_between_comments_and_blocks_preserved_issue165 + ), + ( + "alias_references_do_not_duplicate_rhs_issue167", + ground_truth_alias_references_do_not_duplicate_rhs_issue167, + idempotency_alias_references_do_not_duplicate_rhs_issue167 + ), + ( + "multiline_record_comments_preserved_issue168", + ground_truth_multiline_record_comments_preserved_issue168, + idempotency_multiline_record_comments_preserved_issue168 + ), ); diff --git a/tests/main.rs b/tests/main.rs index 0564f9f..fb6515f 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -4,8 +4,7 @@ use std::{fs, io::Write, path::PathBuf, process::Command}; use tempfile::tempdir; const INVALID: &str = "# beginning of script comment - -let one = 1 +let one = 1 "; const VALID: &str = "# beginning of script comment let one = 1