Skip to content

Batch fix issues #147

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fdncred merged 3 commits into from
Mar 28, 2026
Merged

Batch fix issues #147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
79b98b9
implement issue126/127/136-146 regressions
fdncred Mar 28, 2026
542b6cb
tweaks
fdncred Mar 28, 2026
b64fd83
fmt
fdncred Mar 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 91 additions & 8 deletions src/formatting/blocks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,10 @@ impl<'a> Formatter<'a> {
}

if i < num_pipelines - 1 {
let separator_newlines = if self.indent_level == 0 && self.config.margin > 1 {
self.config.margin.saturating_add(1)
} else {
1
};
let separator_newlines = self.separator_newlines_between_top_level_pipelines(
pipeline,
&block.pipelines[i + 1],
);

for _ in 0..separator_newlines {
self.newline();
Expand All @@ -52,6 +51,81 @@ impl<'a> Formatter<'a> {
}
}

/// Decide how many newline characters to emit between adjacent pipelines.
///
/// At top-level this respects `margin` while preserving author-intent groups
/// for `margin = 1`, and keeps adjacent `use` statements compact.
fn separator_newlines_between_top_level_pipelines(
&self,
current: &nu_protocol::ast::Pipeline,
next: &nu_protocol::ast::Pipeline,
) -> usize {
if self.indent_level != 0 {
return 1;
}

if self.is_use_pipeline(current) && self.is_use_pipeline(next) {
return 1;
}

if self.config.margin == 1 {
let current_end = current
.elements
.last()
.map_or(0, |element| self.get_element_end_pos(element));
let next_start = next
.elements
.first()
.map_or(current_end, |element| element.expr.span.start);

if current_end < next_start {
let between = &self.source[current_end..next_start];
if between.contains(&b'#') {
return 1;
}

let mut previous_newline: Option<usize> = None;
let mut has_blank_line = false;
for (idx, byte) in between.iter().enumerate() {
if *byte == b'\n' {
if let Some(prev) = previous_newline {
if between[prev + 1..idx]
.iter()
.all(|b| b.is_ascii_whitespace())
{
has_blank_line = true;
break;
}
}
previous_newline = Some(idx);
}
}

if has_blank_line {
return 2;
}
}

return 1;
}

self.config.margin.saturating_add(1)
}

/// Whether a pipeline is a top-level `use` command.
fn is_use_pipeline(&self, pipeline: &nu_protocol::ast::Pipeline) -> bool {
let Some(first) = pipeline.elements.first() else {
return false;
};

let Expr::Call(call) = &first.expr.expr else {
return false;
};

let decl = self.working_set.get_decl(call.decl_id);
matches!(decl.name(), "use" | "export use")
}

/// Get the end position of a pipeline element, including redirections.
fn get_element_end_pos(&self, element: &PipelineElement) -> usize {
element
Expand Down Expand Up @@ -98,7 +172,7 @@ impl<'a> Formatter<'a> {
}

/// Format a single pipeline element (expression + optional redirection).
fn format_pipeline_element(&mut self, element: &PipelineElement) {
pub(super) fn format_pipeline_element(&mut self, element: &PipelineElement) {
self.format_expression(&element.expr);
if let Some(ref redirection) = element.redirection {
self.format_redirection(redirection);
Expand Down Expand Up @@ -173,6 +247,7 @@ impl<'a> Formatter<'a> {
&& block.pipelines[0].elements.len() == 1
&& !self.block_has_nested_structures(block)
&& !source_has_newline;
let has_comments_in_block_span = self.has_comments_in_span(span.start, span.end);
let preserve_compact_record_like =
with_braces && is_simple && self.block_expression_looks_like_compact_record(span);

Expand All @@ -185,7 +260,13 @@ impl<'a> Formatter<'a> {
self.write(" ");
}
} else if block.pipelines.is_empty() {
if with_braces {
if with_braces && has_comments_in_block_span {
self.newline();
self.indent_level += 1;
self.write_comments_before(span.end.saturating_sub(1));
self.indent_level -= 1;
self.write_indent();
} else if with_braces {
self.write(" ");
}
} else {
Expand Down Expand Up @@ -306,9 +387,11 @@ impl<'a> Formatter<'a> {
self.write("|");

let block = self.working_set.get_block(block_id);
let has_comments = self.has_comments_in_span(span.start, span.end);
let is_simple = block.pipelines.len() == 1
&& block.pipelines[0].elements.len() == 1
&& !self.block_has_nested_structures(block);
&& !self.block_has_nested_structures(block)
&& !has_comments;

if is_simple {
self.space();
Expand Down
157 changes: 154 additions & 3 deletions src/formatting/calls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +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 LET_COMMANDS: &[&str] = &["let", "let-env", "mut", "const"];
pub(super) const LET_COMMANDS: &[&str] = &["let", "let-env", "mut", "const", "export const"];

impl<'a> Formatter<'a> {
// ─────────────────────────────────────────────────────────────────────────
Expand All @@ -32,6 +32,11 @@ impl<'a> Formatter<'a> {
let decl_name = decl.name();
let cmd_type = Self::classify_command(decl_name);

if self.should_wrap_call_multiline(call, &cmd_type) {
self.format_wrapped_call(call);
return;
}

// Write command name
if call.head.end != 0 {
self.write_span(call.head);
Expand All @@ -52,6 +57,86 @@ impl<'a> Formatter<'a> {
}
}

/// Decide if a call should be emitted as a parenthesized multiline call.
fn should_wrap_call_multiline(
&self,
call: &nu_protocol::ast::Call,
cmd_type: &CommandType,
) -> bool {
if !matches!(cmd_type, CommandType::Regular) || call.arguments.len() < 3 {
return false;
}

if !call.arguments.iter().all(|arg| {
matches!(
arg,
Argument::Positional(_) | Argument::Unknown(_) | Argument::Spread(_)
)
}) {
return false;
}

let end = call
.arguments
.iter()
.map(|arg| match arg {
Argument::Positional(expr) | Argument::Unknown(expr) | Argument::Spread(expr) => {
expr.span.end
}
Argument::Named(named) => named
.2
.as_ref()
.map_or(named.0.span.end, |value| value.span.end),
})
.max()
.unwrap_or(call.head.end);

if call.head.start >= end || end > self.source.len() {
return false;
}

let source_span = &self.source[call.head.start..end];
if source_span.contains(&b'\n') {
return false;
}

source_span.len() > self.config.line_length
}

/// Format a long regular call as:
///
/// `(cmd\n arg1\n arg2\n)`
fn format_wrapped_call(&mut self, call: &nu_protocol::ast::Call) {
self.write("(");
if call.head.end != 0 {
self.write_span(call.head);
}
self.newline();
self.indent_level += 1;

for arg in &call.arguments {
self.write_indent();
match arg {
Argument::Positional(expr) | Argument::Unknown(expr) => {
self.format_expression(expr);
}
Argument::Spread(expr) => {
self.write("...");
self.format_expression(expr);
}
Argument::Named(_) => {
// Guarded out by should_wrap_call_multiline.
self.format_call_argument(arg, &CommandType::Regular);
}
}
self.newline();
}

self.indent_level -= 1;
self.write_indent();
self.write(")");
}

/// Format `let`/`mut`/`const` calls while preserving explicit type annotations.
pub(super) fn format_let_call(&mut self, call: &nu_protocol::ast::Call) {
let positional: Vec<&Expression> = call
Expand Down Expand Up @@ -290,7 +375,7 @@ impl<'a> Formatter<'a> {
Expr::VarDecl(_) => self.format_expression(positional),
Expr::Subexpression(block_id) => {
self.write("= ");
self.format_subexpression(*block_id, positional.span);
self.format_assignment_subexpression(*block_id, positional.span);
}
Expr::Block(block_id) => {
self.write("= ");
Expand All @@ -304,6 +389,37 @@ impl<'a> Formatter<'a> {
}
}

/// Format let-assignment subexpressions, flattening redundant outer
/// parentheses around pipeline-leading subexpressions such as
/// `((pwd) | path join ...)`.
fn format_assignment_subexpression(
&mut self,
block_id: nu_protocol::BlockId,
span: nu_protocol::Span,
) {
let block = self.working_set.get_block(block_id);
if block.pipelines.len() == 1 {
let pipeline = &block.pipelines[0];
if pipeline.elements.len() > 1
&& matches!(pipeline.elements[0].expr.expr, Expr::Subexpression(_))
{
if let Expr::Subexpression(inner_id) = &pipeline.elements[0].expr.expr {
let inner = self.working_set.get_block(*inner_id);
if inner.pipelines.len() == 1 && inner.pipelines[0].elements.len() == 1 {
self.format_pipeline_element(&inner.pipelines[0].elements[0]);
for element in pipeline.elements.iter().skip(1) {
self.write(" | ");
self.format_pipeline_element(element);
}
return;
}
}
}
}

self.format_subexpression(block_id, span);
}

/// Format an external call (e.g. `^git status`).
pub(super) fn format_external_call(&mut self, head: &Expression, args: &[ExternalArgument]) {
// Preserve explicit `^` prefix
Expand Down Expand Up @@ -335,7 +451,11 @@ impl<'a> Formatter<'a> {
+ sig.optional_positional.len()
+ sig.named.iter().filter(|f| f.long != "help").count()
+ usize::from(sig.rest_positional.is_some());
let has_multiline = param_count > 3;
let has_multiline = if self.should_keep_simple_signature_inline(sig) {
false
} else {
param_count > 3
};

if has_multiline {
self.newline();
Expand Down Expand Up @@ -451,6 +571,37 @@ impl<'a> Formatter<'a> {
}
}

/// Keep simple required-positional signatures inline when they fit the
/// configured line length.
fn should_keep_simple_signature_inline(&self, sig: &Signature) -> bool {
if sig.required_positional.is_empty()
|| !sig.optional_positional.is_empty()
|| sig.rest_positional.is_some()
|| !sig.input_output_types.is_empty()
|| sig.named.iter().any(|flag| flag.long != "help")
{
return false;
}

if sig
.required_positional
.iter()
.any(|param| param.shape != SyntaxShape::Any || param.completion.is_some())
{
return false;
}

let inline_len = 2
+ sig
.required_positional
.iter()
.map(|param| param.name.len())
.sum::<usize>()
+ sig.required_positional.len().saturating_sub(1) * 2;

inline_len <= self.config.line_length
}

// ─────────────────────────────────────────────────────────────────────────
// Custom completions and shapes
// ─────────────────────────────────────────────────────────────────────────
Expand Down
11 changes: 10 additions & 1 deletion src/formatting/collections.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ impl<'a> Formatter<'a> {
return;
}

let source_has_newline =
span.end > span.start && self.source[span.start..span.end].contains(&b'\n');

let preserve_compact = self.record_preserve_compact_style(span);

let all_simple = items.iter().all(|item| match item {
Expand All @@ -122,7 +125,9 @@ impl<'a> Formatter<'a> {
// Records with 2+ items and complex values should be multiline when nested
let nested_multiline = self.indent_level > 0 && items.len() >= 2 && has_nested_complex;

if all_simple && items.len() <= 3 && !nested_multiline {
let preserve_multiline_top_level = source_has_newline && self.indent_level == 0;

if all_simple && items.len() <= 3 && !nested_multiline && !preserve_multiline_top_level {
// Inline format
let record_start = self.output.len();
self.write("{");
Expand Down Expand Up @@ -278,6 +283,10 @@ impl<'a> Formatter<'a> {
for (pattern, expr) in matches {
self.write_indent();
self.format_match_pattern(pattern);
if let Some(guard) = &pattern.guard {
self.write(" if ");
self.format_expression(guard);
}
self.write(" => ");
self.format_block_or_expr(expr);
self.newline();
Expand Down
Loading
Loading