diff --git a/src/formatting/blocks.rs b/src/formatting/blocks.rs index 51c1351..c5660e7 100644 --- a/src/formatting/blocks.rs +++ b/src/formatting/blocks.rs @@ -11,6 +11,12 @@ use nu_protocol::{ Span, }; +#[derive(Copy, Clone, Eq, PartialEq)] +enum LetFamily { + Variable, + Constant, +} + impl<'a> Formatter<'a> { // ───────────────────────────────────────────────────────────────────────── // Block and pipeline formatting @@ -60,50 +66,41 @@ impl<'a> Formatter<'a> { current: &nu_protocol::ast::Pipeline, next: &nu_protocol::ast::Pipeline, ) -> usize { - if self.indent_level != 0 { + if self.indent_level != 0 && self.config.margin > 1 { return 1; } - if self.is_use_pipeline(current) && self.is_use_pipeline(next) { + if self.indent_level == 0 && 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; - } + if self.indent_level == 0 { + if let (Some(current_family), Some(next_family)) = ( + self.pipeline_let_family(current), + self.pipeline_let_family(next), + ) { + if current_family == next_family { + if self.pipeline_call_span_has_newline(current) + || self.pipeline_call_span_has_newline(next) + { + return self.config.margin.saturating_add(1); + } - let mut previous_newline: Option = 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 self.has_comment_between_pipelines(current, next) { + // Let the margin/blank-line logic below decide spacing for + // comment-delimited groups. + } else { + return 1; } + } else { + return self.config.margin.saturating_add(1); } + } + } - if has_blank_line { - return 2; - } + if self.config.margin == 1 { + if self.has_blank_line_between_pipelines(current, next) { + return 2; } return 1; @@ -114,16 +111,101 @@ impl<'a> Formatter<'a> { /// Whether a pipeline is a top-level `use` command. fn is_use_pipeline(&self, pipeline: &nu_protocol::ast::Pipeline) -> bool { + matches!( + self.pipeline_decl_name(pipeline), + Some("use" | "export use") + ) + } + + fn pipeline_decl_name(&self, pipeline: &nu_protocol::ast::Pipeline) -> Option<&str> { + let first = pipeline.elements.first()?; + let Expr::Call(call) = &first.expr.expr else { + return None; + }; + + Some(self.working_set.get_decl(call.decl_id).name()) + } + + fn pipeline_let_family(&self, pipeline: &nu_protocol::ast::Pipeline) -> Option { + match self.pipeline_decl_name(pipeline)? { + "let" | "let-env" | "mut" => Some(LetFamily::Variable), + "const" | "export const" => Some(LetFamily::Constant), + _ => None, + } + } + + fn has_blank_line_between_pipelines( + &self, + current: &nu_protocol::ast::Pipeline, + next: &nu_protocol::ast::Pipeline, + ) -> bool { + 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 { + return false; + } + + let between = &self.source[current_end..next_start]; + let mut previous_newline: Option = None; + for (idx, byte) in between.iter().enumerate() { + if *byte != b'\n' { + continue; + } + + if let Some(prev) = previous_newline { + if between[prev + 1..idx] + .iter() + .all(|b| b.is_ascii_whitespace()) + { + return true; + } + } + + previous_newline = Some(idx); + } + + false + } + + fn has_comment_between_pipelines( + &self, + current: &nu_protocol::ast::Pipeline, + next: &nu_protocol::ast::Pipeline, + ) -> bool { + 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); + + current_end < next_start && self.source[current_end..next_start].contains(&b'#') + } + + fn pipeline_call_span_has_newline(&self, pipeline: &nu_protocol::ast::Pipeline) -> bool { let Some(first) = pipeline.elements.first() else { return false; }; + let Some(last) = pipeline.elements.last() 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") + let start = call.head.start; + let end = self.get_element_end_pos(last); + start < end && self.source[start..end].contains(&b'\n') } /// Get the end position of a pipeline element, including redirections. @@ -151,11 +233,14 @@ impl<'a> Formatter<'a> { } // Detect whether the source places pipeline elements on separate lines. - let is_multiline = pipeline.elements.windows(2).any(|pair| { + let source_is_multiline = pipeline.elements.windows(2).any(|pair| { let prev_end = self.get_element_end_pos(&pair[0]); let next_start = pair[1].expr.span.start; prev_end < next_start && self.source[prev_end..next_start].contains(&b'\n') }); + let is_multiline = source_is_multiline + || (self.force_pipeline_multiline_depth > 0 + && self.pipeline_requires_multiline(pipeline)); for (i, element) in pipeline.elements.iter().enumerate() { if i > 0 { @@ -227,6 +312,10 @@ impl<'a> Formatter<'a> { span: Span, with_braces: bool, ) { + if with_braces && self.try_format_pipe_closure_block_from_span(span) { + return; + } + let block = self.working_set.get_block(block_id); let source_has_newline = with_braces @@ -313,6 +402,69 @@ impl<'a> Formatter<'a> { .any(|e| self.expr_is_complex(&e.expr)) } + /// Decide whether a pipeline should be expanded across multiple lines. + pub(super) fn pipeline_requires_multiline( + &self, + pipeline: &nu_protocol::ast::Pipeline, + ) -> bool { + if pipeline.elements.len() > 3 { + return true; + } + + if pipeline + .elements + .iter() + .any(|element| self.expr_contains_nested_pipeline(&element.expr)) + { + return true; + } + + let Some(first) = pipeline.elements.first() else { + return false; + }; + let Some(last) = pipeline.elements.last() else { + return false; + }; + + let start = first.expr.span.start; + let end = self.get_element_end_pos(last); + if start >= end { + return false; + } + + let estimated_inline_len = self.config.indent * self.indent_level + (end - start); + estimated_inline_len > self.config.line_length + } + + fn expr_contains_nested_pipeline(&self, expr: &Expression) -> bool { + match &expr.expr { + Expr::Subexpression(block_id) | Expr::Block(block_id) | Expr::Closure(block_id) => { + let block = self.working_set.get_block(*block_id); + block.pipelines.iter().any(|pipeline| { + pipeline.elements.len() > 1 + || pipeline + .elements + .iter() + .any(|element| self.expr_contains_nested_pipeline(&element.expr)) + }) + } + Expr::Call(call) => call.arguments.iter().any(|arg| match arg { + Argument::Positional(inner) + | Argument::Unknown(inner) + | Argument::Spread(inner) => self.expr_contains_nested_pipeline(inner), + Argument::Named(named) => named + .2 + .as_ref() + .is_some_and(|inner| self.expr_contains_nested_pipeline(inner)), + }), + Expr::Keyword(keyword) => self.expr_contains_nested_pipeline(&keyword.expr), + Expr::BinaryOp(lhs, _, rhs) => { + self.expr_contains_nested_pipeline(lhs) || self.expr_contains_nested_pipeline(rhs) + } + _ => false, + } + } + /// Check if an expression is complex enough to warrant multiline formatting. pub(super) fn expr_is_complex(&self, expr: &Expression) -> bool { match &expr.expr { @@ -363,26 +515,7 @@ impl<'a> Formatter<'a> { }; self.write("{|"); - - // Normalise parameter whitespace - let params = &content[first_pipe + 1..second_pipe]; - let mut params_iter = params.split(|&b| b == b',').peekable(); - - while let Some(param) = params_iter.next() { - let mut sub_parts = param.splitn(2, |&b| b == b':'); - - if let (Some(param_name), Some(type_hint)) = (sub_parts.next(), sub_parts.next()) { - self.write_bytes(param_name.trim_ascii()); - self.write_bytes(b": "); - self.write_bytes(type_hint.trim_ascii()); - } else { - self.write_bytes(param.trim_ascii()); - } - - if params_iter.peek().is_some() { - self.write_bytes(b", "); - } - } + self.write_normalized_closure_params(&content[first_pipe + 1..second_pipe]); self.write("|"); @@ -407,4 +540,66 @@ impl<'a> Formatter<'a> { self.write("}"); } } + + fn write_normalized_closure_params(&mut self, params: &[u8]) { + let mut params_iter = params.split(|&b| b == b',').peekable(); + + while let Some(param) = params_iter.next() { + let mut sub_parts = param.splitn(2, |&b| b == b':'); + + if let (Some(param_name), Some(type_hint)) = (sub_parts.next(), sub_parts.next()) { + self.write_bytes(param_name.trim_ascii()); + self.write_bytes(b": "); + self.write_bytes(type_hint.trim_ascii()); + } else { + self.write_bytes(param.trim_ascii()); + } + + if params_iter.peek().is_some() { + self.write_bytes(b", "); + } + } + } + + /// Normalise closure-like blocks parsed as regular block expressions, + /// such as `{ |line| $line }`. + fn try_format_pipe_closure_block_from_span(&mut self, span: Span) -> bool { + if span.end <= span.start + 2 || span.end > self.source.len() { + return false; + } + + let raw = &self.source[span.start..span.end]; + if !raw.starts_with(b"{") || !raw.ends_with(b"}") { + return false; + } + + let inner = raw[1..raw.len() - 1].trim_ascii(); + if inner.first() != Some(&b'|') || inner.contains(&b'\n') { + return false; + } + + let Some(second_pipe_rel) = inner[1..] + .iter() + .position(|byte| *byte == b'|') + .map(|pos| pos + 1) + else { + return false; + }; + + let params = &inner[1..second_pipe_rel]; + let body = inner[second_pipe_rel + 1..].trim_ascii(); + + self.write("{|"); + self.write_normalized_closure_params(params); + self.write("|"); + + if !body.is_empty() { + self.space(); + self.write_bytes(body); + self.write(" "); + } + + self.write("}"); + true + } } diff --git a/src/formatting/calls.rs b/src/formatting/calls.rs index a95ffba..75341fe 100644 --- a/src/formatting/calls.rs +++ b/src/formatting/calls.rs @@ -179,11 +179,18 @@ impl<'a> Formatter<'a> { self.write(" = "); match &rhs.expr { - Expr::Block(block_id) | Expr::Subexpression(block_id) => { + Expr::Subexpression(block_id) => { + self.format_assignment_subexpression(*block_id, rhs.span); + } + Expr::Block(block_id) => { let block = self.working_set.get_block(*block_id); self.format_block(block); } - _ => self.format_expression(rhs), + _ => { + if !self.try_write_redundant_parenthesized_pipeline_rhs(rhs) { + self.format_expression(rhs); + } + } } for extra in positional.iter().skip(2) { @@ -338,7 +345,11 @@ impl<'a> Formatter<'a> { self.format_block_or_expr(positional); } CommandType::Let => self.format_let_argument(positional), - CommandType::Regular => self.format_expression(positional), + CommandType::Regular => { + if !self.try_format_closure_like_span(positional.span) { + self.format_expression(positional); + } + } } } @@ -415,6 +426,11 @@ impl<'a> Formatter<'a> { } } } + + if !self.pipeline_requires_multiline(pipeline) { + self.format_block(block); + return; + } } self.format_subexpression(block_id, span); @@ -439,6 +455,100 @@ impl<'a> Formatter<'a> { } } + fn try_write_redundant_parenthesized_pipeline_rhs(&mut self, rhs: &Expression) -> bool { + let raw = self.get_span_content(rhs.span); + let trimmed = raw.trim_ascii(); + if trimmed.len() < 3 || trimmed.contains(&b'\n') { + return false; + } + + if !(trimmed.starts_with(b"(") && trimmed.ends_with(b")") && trimmed.contains(&b'|')) { + return false; + } + + let inner = &trimmed[1..trimmed.len() - 1]; + let inner = inner.trim_ascii(); + + // Keep explicit wrappers for external-command assignments. + if inner.starts_with(b"^") { + return false; + } + + if inner.is_empty() { + return false; + } + + self.write_bytes(inner); + true + } + + fn try_format_closure_like_span(&mut self, span: nu_protocol::Span) -> bool { + if span.end <= span.start + 2 || span.end > self.source.len() { + return false; + } + + let raw = &self.source[span.start..span.end]; + let trimmed = raw.trim_ascii(); + if trimmed.len() < 4 || trimmed.contains(&b'\n') { + return false; + } + + if trimmed + .get(1) + .is_none_or(|byte| !byte.is_ascii_whitespace()) + { + return false; + } + + if !(trimmed.starts_with(b"{") && trimmed.ends_with(b"}")) { + return false; + } + + let inner = trimmed[1..trimmed.len() - 1].trim_ascii(); + if inner.first() != Some(&b'|') { + return false; + } + + let Some(second_pipe) = inner[1..] + .iter() + .position(|byte| *byte == b'|') + .map(|pos| pos + 1) + else { + return false; + }; + + let params = &inner[1..second_pipe]; + let body = inner[second_pipe + 1..].trim_ascii(); + + self.write("{|"); + let mut params_iter = params.split(|&b| b == b',').peekable(); + while let Some(param) = params_iter.next() { + let mut sub_parts = param.splitn(2, |&b| b == b':'); + + if let (Some(param_name), Some(type_hint)) = (sub_parts.next(), sub_parts.next()) { + self.write_bytes(param_name.trim_ascii()); + self.write_bytes(b": "); + self.write_bytes(type_hint.trim_ascii()); + } else { + self.write_bytes(param.trim_ascii()); + } + + if params_iter.peek().is_some() { + self.write_bytes(b", "); + } + } + self.write("|"); + + if !body.is_empty() { + self.space(); + self.write_bytes(body); + self.write(" "); + } + + self.write("}"); + true + } + // ───────────────────────────────────────────────────────────────────────── // Signature formatting // ───────────────────────────────────────────────────────────────────────── diff --git a/src/formatting/collections.rs b/src/formatting/collections.rs index 787987f..ec26962 100644 --- a/src/formatting/collections.rs +++ b/src/formatting/collections.rs @@ -22,12 +22,18 @@ impl<'a> Formatter<'a> { } let uses_commas = self.list_uses_commas(items); + let source_has_newline = self.list_has_source_newline(items); let all_simple = items.iter().all(|item| match item { ListItem::Item(expr) | ListItem::Spread(_, expr) => self.is_simple_expr(expr), }); - if all_simple && items.len() <= 5 { + 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); + + if should_inline { self.write("["); for (i, item) in items.iter().enumerate() { if i > 0 { @@ -44,11 +50,35 @@ impl<'a> Formatter<'a> { self.write("["); self.newline(); self.indent_level += 1; - for item in items { + + let mut idx = 0; + while idx < items.len() { self.write_indent(); - self.format_list_item(item); + + if idx + 1 < items.len() { + let current = &items[idx]; + let next = &items[idx + 1]; + if self.should_pair_flag_value_items(current, next) + && self.paired_list_items_fit_on_line(current, next, uses_commas) + { + self.format_list_item(current); + if uses_commas { + self.write(", "); + } else { + self.write(" "); + } + self.format_list_item(next); + self.newline(); + idx += 2; + continue; + } + } + + self.format_list_item(&items[idx]); self.newline(); + idx += 1; } + self.indent_level -= 1; self.write_indent(); self.write("]"); @@ -61,13 +91,9 @@ impl<'a> Formatter<'a> { return false; } - let item_bounds = |item: &ListItem| match item { - ListItem::Item(expr) | ListItem::Spread(_, expr) => (expr.span.start, expr.span.end), - }; - - let (_, mut prev_end) = item_bounds(&items[0]); + let (_, mut prev_end) = self.list_item_bounds(&items[0]); for item in items.iter().skip(1) { - let (start, end) = item_bounds(item); + let (start, end) = self.list_item_bounds(item); if start > prev_end && self.source[prev_end..start].contains(&b',') { return true; } @@ -88,6 +114,83 @@ impl<'a> Formatter<'a> { } } + fn list_item_bounds(&self, item: &ListItem) -> (usize, usize) { + match item { + ListItem::Item(expr) | ListItem::Spread(_, expr) => (expr.span.start, expr.span.end), + } + } + + fn list_has_source_newline(&self, items: &[ListItem]) -> bool { + if items.len() < 2 { + return false; + } + + let (_, mut prev_end) = self.list_item_bounds(&items[0]); + for item in items.iter().skip(1) { + let (start, end) = self.list_item_bounds(item); + if prev_end < start && self.source[prev_end..start].contains(&b'\n') { + return true; + } + prev_end = end; + } + + false + } + + fn should_pair_flag_value_items(&self, current: &ListItem, next: &ListItem) -> bool { + self.list_item_is_flag_string(current) && !self.list_item_is_flag_string(next) + } + + fn list_item_is_flag_string(&self, item: &ListItem) -> bool { + let expr = match item { + ListItem::Item(expr) => expr, + ListItem::Spread(_, _) => return false, + }; + + if !matches!(expr.expr, Expr::String(_)) { + return false; + } + + let raw = self.get_span_content(expr.span); + let trimmed = raw.trim_ascii(); + if trimmed.len() < 3 || trimmed.first() != Some(&b'"') || trimmed.last() != Some(&b'"') { + return false; + } + + let inner = &trimmed[1..trimmed.len() - 1]; + inner.starts_with(b"-") + } + + fn paired_list_items_fit_on_line( + &self, + current: &ListItem, + next: &ListItem, + uses_commas: bool, + ) -> bool { + let mut probe = Formatter::new( + self.source, + self.working_set, + self.config, + self.allow_compact_recovered_record_style, + ); + probe.format_list_item(current); + let left_len = probe.output.len(); + + let mut probe = Formatter::new( + self.source, + self.working_set, + self.config, + self.allow_compact_recovered_record_style, + ); + probe.format_list_item(next); + let right_len = probe.output.len(); + + let separator_len = if uses_commas { 2 } else { 1 }; + let indent_len = self.config.indent * self.indent_level; + + indent_len + left_len + separator_len + right_len <= self.config.line_length + } + // ───────────────────────────────────────────────────────────────────────── // Records // ───────────────────────────────────────────────────────────────────────── @@ -276,18 +379,37 @@ impl<'a> Formatter<'a> { /// Format a match block (`match $val { pattern => expr }`). pub(super) fn format_match_block(&mut self, matches: &[(MatchPattern, Expression)]) { + let rendered_lhs: Vec> = matches + .iter() + .map(|(pattern, expr)| self.render_match_arm_lhs(pattern, expr)) + .collect(); + + let should_align = self.should_preserve_match_arm_alignment(matches) + && rendered_lhs.iter().all(|lhs| !lhs.contains(&b'\n')); + let max_lhs_len = if should_align { + rendered_lhs.iter().map(Vec::len).max().unwrap_or(0) + } else { + 0 + }; + self.write("{"); self.newline(); self.indent_level += 1; - for (pattern, expr) in matches { + for ((_pattern, expr), lhs) in matches.iter().zip(rendered_lhs.iter()) { self.write_indent(); - self.format_match_pattern(pattern); - if let Some(guard) = &pattern.guard { - self.write(" if "); - self.format_expression(guard); + self.write_bytes(lhs); + + if should_align { + let spaces = max_lhs_len.saturating_sub(lhs.len()) + 1; + for _ in 0..spaces { + self.output.push(b' '); + } + } else { + self.write(" "); } - self.write(" => "); + + self.write("=> "); self.format_block_or_expr(expr); self.newline(); } @@ -338,4 +460,145 @@ impl<'a> Formatter<'a> { Pattern::IgnoreValue => self.write("_"), } } + + fn render_match_arm_lhs(&self, pattern: &MatchPattern, rhs: &Expression) -> Vec { + let mut probe = Formatter::new( + self.source, + self.working_set, + self.config, + self.allow_compact_recovered_record_style, + ); + probe.format_match_pattern_for_arm(pattern, rhs); + if let Some(guard) = &pattern.guard { + probe.write(" if "); + probe.format_expression(guard); + } + probe.output + } + + fn should_preserve_match_arm_alignment(&self, matches: &[(MatchPattern, Expression)]) -> bool { + let spacing_hints: Vec = matches + .iter() + .filter_map(|(pattern, expr)| self.source_spaces_before_match_arrow(pattern, expr)) + .collect(); + + if spacing_hints.len() < 2 || spacing_hints.iter().all(|spaces| *spaces <= 1) { + return false; + } + + let arrow_columns: Vec = matches + .iter() + .filter_map(|(pattern, expr)| self.source_match_arrow_column(pattern, expr)) + .collect(); + + if arrow_columns.len() < 2 { + return false; + } + + let first = arrow_columns[0]; + arrow_columns.iter().all(|col| *col == first) + } + + fn source_match_arrow_column( + &self, + pattern: &MatchPattern, + expr: &Expression, + ) -> Option { + if pattern.span.end >= expr.span.start || expr.span.start > self.source.len() { + return None; + } + + let between = &self.source[pattern.span.end..expr.span.start]; + let arrow_idx = between.windows(2).position(|pair| pair == b"=>")?; + let line_start = self.source[..pattern.span.start] + .iter() + .rposition(|byte| *byte == b'\n') + .map_or(0, |idx| idx + 1); + + Some(pattern.span.end.saturating_sub(line_start) + arrow_idx) + } + + fn source_spaces_before_match_arrow( + &self, + pattern: &MatchPattern, + expr: &Expression, + ) -> Option { + if pattern.span.end >= expr.span.start || expr.span.start > self.source.len() { + return None; + } + + let between = &self.source[pattern.span.end..expr.span.start]; + let arrow_idx = between.windows(2).position(|pair| pair == b"=>")?; + let before_arrow = &between[..arrow_idx]; + + Some( + before_arrow + .iter() + .rev() + .take_while(|byte| byte.is_ascii_whitespace()) + .count(), + ) + } + + fn format_match_pattern_for_arm(&mut self, pattern: &MatchPattern, rhs: &Expression) { + if let Pattern::Expression(expr) = &pattern.pattern { + if self.should_unquote_identifier_safe_match_pattern(expr, rhs) { + let raw = self.get_span_content(expr.span); + let trimmed = raw.trim_ascii(); + let inner = &trimmed[1..trimmed.len() - 1]; + self.write_bytes(inner); + return; + } + } + + self.format_match_pattern(pattern); + } + + fn should_unquote_identifier_safe_match_pattern( + &self, + expr: &Expression, + rhs: &Expression, + ) -> bool { + if !matches!(expr.expr, Expr::String(_)) { + return false; + } + + if matches!( + rhs.expr, + Expr::Subexpression(_) | Expr::Block(_) | Expr::Closure(_) + ) { + return false; + } + + let rhs_raw = self.get_span_content(rhs.span); + let rhs_trimmed = rhs_raw.trim_ascii(); + if rhs_trimmed.starts_with(b"(") && rhs_trimmed.ends_with(b")") { + return false; + } + + let raw = self.get_span_content(expr.span); + let trimmed = raw.trim_ascii(); + if trimmed.len() < 3 || trimmed.first() != Some(&b'"') || trimmed.last() != Some(&b'"') { + return false; + } + + let inner = &trimmed[1..trimmed.len() - 1]; + if inner.is_empty() || inner.contains(&b'\\') { + return false; + } + + let first = inner[0]; + if !(first.is_ascii_alphabetic() || first == b'_') { + return false; + } + + if !inner + .iter() + .all(|byte| byte.is_ascii_alphanumeric() || *byte == b'_') + { + return false; + } + + true + } } diff --git a/src/formatting/expressions.rs b/src/formatting/expressions.rs index 6728dc5..71c48f6 100644 --- a/src/formatting/expressions.rs +++ b/src/formatting/expressions.rs @@ -35,7 +35,9 @@ impl<'a> Formatter<'a> { } Expr::Garbage => { - if !self.try_write_redundant_pipeline_subexpr_without_outer_parens(expr) { + if !(self.try_write_redundant_pipeline_subexpr_without_outer_parens(expr) + || self.try_write_spacing_normalized_pipe_closure_garbage(expr)) + { self.write_expr_span(expr); } } @@ -289,14 +291,17 @@ impl<'a> Formatter<'a> { self.write("("); let is_simple = !source_has_newline && block.pipelines.len() == 1 - && block.pipelines[0].elements.len() <= 3; + && !self.pipeline_requires_multiline(&block.pipelines[0]); if is_simple { self.format_block(block); } else { self.newline(); self.indent_level += 1; + self.force_pipeline_multiline_depth += 1; self.format_block(block); + self.force_pipeline_multiline_depth = + self.force_pipeline_multiline_depth.saturating_sub(1); self.newline(); self.indent_level -= 1; self.write_indent(); @@ -366,6 +371,62 @@ impl<'a> Formatter<'a> { true } + fn try_write_spacing_normalized_pipe_closure_garbage(&mut self, expr: &Expression) -> bool { + let raw = self.get_span_content(expr.span); + let trimmed = raw.trim_ascii(); + if trimmed.len() < 4 || trimmed.contains(&b'\n') { + return false; + } + + if !(trimmed.starts_with(b"{") && trimmed.ends_with(b"}")) { + return false; + } + + let inner = trimmed[1..trimmed.len() - 1].trim_ascii(); + if inner.first() != Some(&b'|') { + return false; + } + + let Some(second_pipe) = inner[1..] + .iter() + .position(|byte| *byte == b'|') + .map(|pos| pos + 1) + else { + return false; + }; + + let params = &inner[1..second_pipe]; + let body = inner[second_pipe + 1..].trim_ascii(); + + self.write("{|"); + let mut params_iter = params.split(|&b| b == b',').peekable(); + while let Some(param) = params_iter.next() { + let mut sub_parts = param.splitn(2, |&b| b == b':'); + + if let (Some(param_name), Some(type_hint)) = (sub_parts.next(), sub_parts.next()) { + self.write_bytes(param_name.trim_ascii()); + self.write_bytes(b": "); + self.write_bytes(type_hint.trim_ascii()); + } else { + self.write_bytes(param.trim_ascii()); + } + + if params_iter.peek().is_some() { + self.write_bytes(b", "); + } + } + self.write("|"); + + if !body.is_empty() { + self.space(); + self.write_bytes(body); + self.write(" "); + } + + self.write("}"); + true + } + /// Check if an expression is a simple primitive (used by collection /// formatting to decide inline vs. multiline layout). pub(super) fn is_simple_expr(&self, expr: &Expression) -> bool { @@ -377,6 +438,7 @@ impl<'a> Formatter<'a> { | Expr::RawString(_) | Expr::Nothing | Expr::Var(_) + | Expr::StringInterpolation(_) | Expr::Filepath(_, _) | Expr::Directory(_, _) | Expr::GlobPattern(_, _) diff --git a/src/formatting/mod.rs b/src/formatting/mod.rs index 3a9caf9..3300184 100644 --- a/src/formatting/mod.rs +++ b/src/formatting/mod.rs @@ -70,6 +70,9 @@ pub(crate) struct Formatter<'a> { /// Optional upper boundary for inline comment capture inside /// delimited contexts (e.g. subexpressions). pub(crate) inline_comment_upper_bound: Option, + /// Force multiline pipeline emission in scoped contexts such as + /// multiline subexpressions. + pub(crate) force_pipeline_multiline_depth: usize, } /// Command types for formatting purposes. @@ -106,6 +109,7 @@ impl<'a> Formatter<'a> { preserve_subexpr_parens_depth: 0, allow_compact_recovered_record_style, inline_comment_upper_bound: None, + force_pipeline_multiline_depth: 0, } } @@ -286,7 +290,111 @@ fn format_inner_with_options(contents: &[u8], config: &Config) -> Result formatter.write_comments_before(contents.len()); } - Ok(formatter.finish()) + Ok(postprocess_formatted_output(formatter.finish())) +} + +fn postprocess_formatted_output(output: Vec) -> Vec { + let mut changed = false; + let text = String::from_utf8_lossy(&output); + let mut rebuilt = String::with_capacity(text.len()); + + for line in text.split_inclusive('\n') { + let (line_body, line_end) = match line.strip_suffix('\n') { + Some(body) => (body, "\n"), + None => (line, ""), + }; + + let mut normalized = normalize_redundant_assignment_pipeline_parens(line_body); + let closure_normalized = normalize_closure_pipe_spacing(&normalized); + if closure_normalized != normalized { + changed = true; + normalized = closure_normalized; + } + + if normalized != line_body { + changed = true; + } + + rebuilt.push_str(&normalized); + rebuilt.push_str(line_end); + } + + if changed { + rebuilt.into_bytes() + } else { + output + } +} + +fn normalize_redundant_assignment_pipeline_parens(line: &str) -> String { + let trimmed_start = line.trim_start(); + let is_let_like = trimmed_start.starts_with("let ") + || trimmed_start.starts_with("let-env ") + || trimmed_start.starts_with("mut ") + || trimmed_start.starts_with("const ") + || trimmed_start.starts_with("export const "); + if !is_let_like { + return line.to_string(); + } + + let Some(eq_idx) = line.find('=') else { + return line.to_string(); + }; + + let rhs = line[eq_idx + 1..].trim_start(); + if !(rhs.starts_with('(') && rhs.ends_with(')') && rhs.contains('|')) { + return line.to_string(); + } + + let inner = rhs[1..rhs.len() - 1].trim(); + if inner.is_empty() || inner.starts_with('^') { + return line.to_string(); + } + + let lhs = line[..eq_idx].trim_end(); + format!("{lhs} = {inner}") +} + +fn normalize_closure_pipe_spacing(line: &str) -> String { + let bytes = line.as_bytes(); + let mut result = Vec::with_capacity(bytes.len()); + let mut idx = 0; + let mut changed = false; + + while idx < bytes.len() { + if bytes[idx] != b'{' { + result.push(bytes[idx]); + idx += 1; + continue; + } + + result.push(b'{'); + idx += 1; + + let spaces_start = idx; + while idx < bytes.len() && bytes[idx].is_ascii_whitespace() && bytes[idx] != b'\n' { + idx += 1; + } + + if idx < bytes.len() && bytes[idx] == b'|' { + let has_second_pipe = bytes[idx + 1..].contains(&b'|'); + let has_closing_brace = bytes[idx + 1..].contains(&b'}'); + if has_second_pipe && has_closing_brace { + if idx > spaces_start { + changed = true; + } + continue; + } + } + + result.extend_from_slice(&bytes[spaces_start..idx]); + } + + if changed { + String::from_utf8(result).unwrap_or_else(|_| line.to_string()) + } else { + line.to_string() + } } /// Make sure there is a newline at the end of a buffer. diff --git a/tests/fixtures/expected/assignment_pipeline_redundant_parens_removed_issue156.nu b/tests/fixtures/expected/assignment_pipeline_redundant_parens_removed_issue156.nu new file mode 100644 index 0000000..509a454 --- /dev/null +++ b/tests/fixtures/expected/assignment_pipeline_redundant_parens_removed_issue156.nu @@ -0,0 +1 @@ +let stdout = $tool_response | get -o stdout diff --git a/tests/fixtures/expected/closure_argument_pipe_spacing_normalized_issue160.nu b/tests/fixtures/expected/closure_argument_pipe_spacing_normalized_issue160.nu new file mode 100644 index 0000000..58f1789 --- /dev/null +++ b/tests/fixtures/expected/closure_argument_pipe_spacing_normalized_issue160.nu @@ -0,0 +1,2 @@ +{|line| $line } +reduce --fold (make-state) {|line| $line } diff --git a/tests/fixtures/expected/comment_spacing_before_toplevel_statements_issue150.nu b/tests/fixtures/expected/comment_spacing_before_toplevel_statements_issue150.nu new file mode 100644 index 0000000..af2f336 --- /dev/null +++ b/tests/fixtures/expected/comment_spacing_before_toplevel_statements_issue150.nu @@ -0,0 +1,28 @@ +# module doc + +# next line comment +use b.nu [colors] +use a.nu # inline comment +use d.nu [thing] + +# constants +const PRIORITY_ERROR: int = 3 +const PRIORITY_INFO: int = 6 +const PRIORITY_DEBUG: int = 7 + +# next multi +# line comment +const VOLATILE_FIELDS: list = ["EXCEPTION" "EXCEPTION_DEBUG"] + +# func comment +def level-to-max-priority [level: string]: nothing -> int { + match ($level | str upcase) { + ERROR => 3 + WARNING => 4 + INFO => 6 + _ => 7 + } +} + +# func comment +def format-timestamp [ts_us: string]: nothing -> nothing { } diff --git a/tests/fixtures/expected/consecutive_let_const_grouping_normalized_issue153.nu b/tests/fixtures/expected/consecutive_let_const_grouping_normalized_issue153.nu new file mode 100644 index 0000000..fa0b3bd --- /dev/null +++ b/tests/fixtures/expected/consecutive_let_const_grouping_normalized_issue153.nu @@ -0,0 +1,7 @@ +let a = "abc" +let b = "xyz" +let c = "foo" + +const x = "abc" +const y = "xyz" +const z = "foo" diff --git a/tests/fixtures/expected/identifier_safe_match_patterns_unquoted_issue157.nu b/tests/fixtures/expected/identifier_safe_match_patterns_unquoted_issue157.nu new file mode 100644 index 0000000..e44b78b --- /dev/null +++ b/tests/fixtures/expected/identifier_safe_match_patterns_unquoted_issue157.nu @@ -0,0 +1,6 @@ +let summary = match $decision { + allow => "ok" + deny => "blocked" + ask => "queued" + _ => $decision +} diff --git a/tests/fixtures/expected/list_flag_value_pairing_preserved_issue151.nu b/tests/fixtures/expected/list_flag_value_pairing_preserved_issue151.nu new file mode 100644 index 0000000..6018228 --- /dev/null +++ b/tests/fixtures/expected/list_flag_value_pairing_preserved_issue151.nu @@ -0,0 +1,7 @@ +let base_args: list = [ + "--no-pager" + "--output" "json" + "--identifier" "inceptool" + "--lines" ($lines | into string) + "--priority" ($max_pri | into string) +] diff --git a/tests/fixtures/expected/margin_respected_inside_nested_blocks_issue154.nu b/tests/fixtures/expected/margin_respected_inside_nested_blocks_issue154.nu new file mode 100644 index 0000000..2fe1303 --- /dev/null +++ b/tests/fixtures/expected/margin_respected_inside_nested_blocks_issue154.nu @@ -0,0 +1,11 @@ +def run [] { + let a = 1 + + let b = 2 + + if true { + let c = 3 + + let d = 4 + } +} diff --git a/tests/fixtures/expected/match_arm_alignment_preserved_issue106.nu b/tests/fixtures/expected/match_arm_alignment_preserved_issue106.nu new file mode 100644 index 0000000..3af0df5 --- /dev/null +++ b/tests/fixtures/expected/match_arm_alignment_preserved_issue106.nu @@ -0,0 +1,5 @@ +match $x { + "rtk" => (rtk hook) + "guardrails" => (guardrails hook) + _ => (allow) +} diff --git a/tests/fixtures/expected/multiline_list_layout_preserved_issue152.nu b/tests/fixtures/expected/multiline_list_layout_preserved_issue152.nu new file mode 100644 index 0000000..3b7af3b --- /dev/null +++ b/tests/fixtures/expected/multiline_list_layout_preserved_issue152.nu @@ -0,0 +1,5 @@ +const VOLATILE_FIELDS: list = [ + "EXCEPTION" + "EXCEPTION_DEBUG" + "taaaaaaaaaab" +] diff --git a/tests/fixtures/expected/nested_closure_indentation_normalized_issue159.nu b/tests/fixtures/expected/nested_closure_indentation_normalized_issue159.nu new file mode 100644 index 0000000..21cf0c5 --- /dev/null +++ b/tests/fixtures/expected/nested_closure_indentation_normalized_issue159.nu @@ -0,0 +1,7 @@ +try { + ^journalctl ...$all_args + | lines + | reduce --fold (make-state) {|line, state| + let entry: record = try { $line | from json } | default {} + } +} diff --git a/tests/fixtures/expected/nested_pipeline_expansion_rules_applied_issue155.nu b/tests/fixtures/expected/nested_pipeline_expansion_rules_applied_issue155.nu new file mode 100644 index 0000000..7804651 --- /dev/null +++ b/tests/fixtures/expected/nested_pipeline_expansion_rules_applied_issue155.nu @@ -0,0 +1,5 @@ +let in_val = ( + $in_parsed + | get -o tool_input + | default ($in_parsed | get -o tool_response | default "") +) diff --git a/tests/fixtures/expected/single_item_list_inline_and_if_layout_preserved_issue158.nu b/tests/fixtures/expected/single_item_list_inline_and_if_layout_preserved_issue158.nu new file mode 100644 index 0000000..758f12f --- /dev/null +++ b/tests/fixtures/expected/single_item_list_inline_and_if_layout_preserved_issue158.nu @@ -0,0 +1,5 @@ +let a = [$"SESSION=($session)"] + +let session_args = if ($session | is-not-empty) { + [$"SESSION=($session)"] +} else { [] } diff --git a/tests/fixtures/input/assignment_pipeline_redundant_parens_removed_issue156.nu b/tests/fixtures/input/assignment_pipeline_redundant_parens_removed_issue156.nu new file mode 100644 index 0000000..b7d4220 --- /dev/null +++ b/tests/fixtures/input/assignment_pipeline_redundant_parens_removed_issue156.nu @@ -0,0 +1 @@ +let stdout = ($tool_response | get -o stdout) diff --git a/tests/fixtures/input/closure_argument_pipe_spacing_normalized_issue160.nu b/tests/fixtures/input/closure_argument_pipe_spacing_normalized_issue160.nu new file mode 100644 index 0000000..135c519 --- /dev/null +++ b/tests/fixtures/input/closure_argument_pipe_spacing_normalized_issue160.nu @@ -0,0 +1,2 @@ +{ |line| $line } +reduce --fold (make-state) { |line| $line } diff --git a/tests/fixtures/input/comment_spacing_before_toplevel_statements_issue150.nu b/tests/fixtures/input/comment_spacing_before_toplevel_statements_issue150.nu new file mode 100644 index 0000000..66c95ab --- /dev/null +++ b/tests/fixtures/input/comment_spacing_before_toplevel_statements_issue150.nu @@ -0,0 +1,28 @@ +# module doc + +# next line comment +use b.nu [colors] +use a.nu # inline comment +use d.nu [thing] + +# constants +const PRIORITY_ERROR: int = 3 +const PRIORITY_INFO: int = 6 +const PRIORITY_DEBUG: int = 7 + +# next multi +# line comment +const VOLATILE_FIELDS: list = ["EXCEPTION" "EXCEPTION_DEBUG"] + +# func comment +def level-to-max-priority [level: string]: nothing -> int { + match ($level | str upcase) { + ERROR => 3 + WARNING => 4 + INFO => 6 + _ => 7 + } +} + +# func comment +def format-timestamp [ts_us: string]: nothing -> nothing { } diff --git a/tests/fixtures/input/consecutive_let_const_grouping_normalized_issue153.nu b/tests/fixtures/input/consecutive_let_const_grouping_normalized_issue153.nu new file mode 100644 index 0000000..25fa51b --- /dev/null +++ b/tests/fixtures/input/consecutive_let_const_grouping_normalized_issue153.nu @@ -0,0 +1,11 @@ +let a = "abc" + +let b = "xyz" + +let c = "foo" + +const x = "abc" + +const y = "xyz" + +const z = "foo" diff --git a/tests/fixtures/input/identifier_safe_match_patterns_unquoted_issue157.nu b/tests/fixtures/input/identifier_safe_match_patterns_unquoted_issue157.nu new file mode 100644 index 0000000..0ebb066 --- /dev/null +++ b/tests/fixtures/input/identifier_safe_match_patterns_unquoted_issue157.nu @@ -0,0 +1,6 @@ +let summary = match $decision { + "allow" => "ok" + "deny" => "blocked" + "ask" => "queued" + _ => $decision +} diff --git a/tests/fixtures/input/list_flag_value_pairing_preserved_issue151.nu b/tests/fixtures/input/list_flag_value_pairing_preserved_issue151.nu new file mode 100644 index 0000000..f996ff5 --- /dev/null +++ b/tests/fixtures/input/list_flag_value_pairing_preserved_issue151.nu @@ -0,0 +1,7 @@ +let base_args: list = [ + "--no-pager" + "--output" "json" + "--identifier" "inceptool" + "--lines" ($lines | into string) + "--priority" ($max_pri | into string) +] diff --git a/tests/fixtures/input/margin_respected_inside_nested_blocks_issue154.nu b/tests/fixtures/input/margin_respected_inside_nested_blocks_issue154.nu new file mode 100644 index 0000000..e0c9955 --- /dev/null +++ b/tests/fixtures/input/margin_respected_inside_nested_blocks_issue154.nu @@ -0,0 +1,11 @@ +def run [] { + let a = 1 + + let b = 2 + + if true { + let c = 3 + + let d = 4 + } +} diff --git a/tests/fixtures/input/match_arm_alignment_preserved_issue106.nu b/tests/fixtures/input/match_arm_alignment_preserved_issue106.nu new file mode 100644 index 0000000..cf9a00f --- /dev/null +++ b/tests/fixtures/input/match_arm_alignment_preserved_issue106.nu @@ -0,0 +1,5 @@ +match $x { + "rtk" => (rtk hook) + "guardrails" => (guardrails hook) + _ => (allow) +} diff --git a/tests/fixtures/input/multiline_list_layout_preserved_issue152.nu b/tests/fixtures/input/multiline_list_layout_preserved_issue152.nu new file mode 100644 index 0000000..0f01f6e --- /dev/null +++ b/tests/fixtures/input/multiline_list_layout_preserved_issue152.nu @@ -0,0 +1,5 @@ +const VOLATILE_FIELDS: list = [ + "EXCEPTION" + "EXCEPTION_DEBUG" + "taaaaaaaaaab" +] diff --git a/tests/fixtures/input/nested_closure_indentation_normalized_issue159.nu b/tests/fixtures/input/nested_closure_indentation_normalized_issue159.nu new file mode 100644 index 0000000..3d8bc78 --- /dev/null +++ b/tests/fixtures/input/nested_closure_indentation_normalized_issue159.nu @@ -0,0 +1,7 @@ +try { + ^journalctl ...$all_args + | lines + | reduce --fold (make-state) {|line, state| + let entry: record = try { $line | from json } | default {} + } +} diff --git a/tests/fixtures/input/nested_pipeline_expansion_rules_applied_issue155.nu b/tests/fixtures/input/nested_pipeline_expansion_rules_applied_issue155.nu new file mode 100644 index 0000000..890ed75 --- /dev/null +++ b/tests/fixtures/input/nested_pipeline_expansion_rules_applied_issue155.nu @@ -0,0 +1 @@ +let in_val = ($in_parsed | get -o tool_input | default ($in_parsed | get -o tool_response | default "")) diff --git a/tests/fixtures/input/single_item_list_inline_and_if_layout_preserved_issue158.nu b/tests/fixtures/input/single_item_list_inline_and_if_layout_preserved_issue158.nu new file mode 100644 index 0000000..02ff709 --- /dev/null +++ b/tests/fixtures/input/single_item_list_inline_and_if_layout_preserved_issue158.nu @@ -0,0 +1,6 @@ +let a = [ + $"SESSION=($session)" +] +let session_args = if ($session | is-not-empty) { [ + $"SESSION=($session)" +] } else { [] } diff --git a/tests/ground_truth.rs b/tests/ground_truth.rs index 963588f..bef2992 100644 --- a/tests/ground_truth.rs +++ b/tests/ground_truth.rs @@ -545,4 +545,64 @@ fixture_tests!( ground_truth_if_else_comment_and_statement_placement_preserved_issue146, idempotency_if_else_comment_and_statement_placement_preserved_issue146 ), + ( + "match_arm_alignment_preserved_issue106", + ground_truth_match_arm_alignment_preserved_issue106, + idempotency_match_arm_alignment_preserved_issue106 + ), + ( + "comment_spacing_before_toplevel_statements_issue150", + ground_truth_comment_spacing_before_toplevel_statements_issue150, + idempotency_comment_spacing_before_toplevel_statements_issue150 + ), + ( + "list_flag_value_pairing_preserved_issue151", + ground_truth_list_flag_value_pairing_preserved_issue151, + idempotency_list_flag_value_pairing_preserved_issue151 + ), + ( + "multiline_list_layout_preserved_issue152", + ground_truth_multiline_list_layout_preserved_issue152, + idempotency_multiline_list_layout_preserved_issue152 + ), + ( + "consecutive_let_const_grouping_normalized_issue153", + ground_truth_consecutive_let_const_grouping_normalized_issue153, + idempotency_consecutive_let_const_grouping_normalized_issue153 + ), + ( + "margin_respected_inside_nested_blocks_issue154", + ground_truth_margin_respected_inside_nested_blocks_issue154, + idempotency_margin_respected_inside_nested_blocks_issue154 + ), + ( + "nested_pipeline_expansion_rules_applied_issue155", + ground_truth_nested_pipeline_expansion_rules_applied_issue155, + idempotency_nested_pipeline_expansion_rules_applied_issue155 + ), + ( + "assignment_pipeline_redundant_parens_removed_issue156", + ground_truth_assignment_pipeline_redundant_parens_removed_issue156, + idempotency_assignment_pipeline_redundant_parens_removed_issue156 + ), + ( + "identifier_safe_match_patterns_unquoted_issue157", + ground_truth_identifier_safe_match_patterns_unquoted_issue157, + idempotency_identifier_safe_match_patterns_unquoted_issue157 + ), + ( + "single_item_list_inline_and_if_layout_preserved_issue158", + ground_truth_single_item_list_inline_and_if_layout_preserved_issue158, + idempotency_single_item_list_inline_and_if_layout_preserved_issue158 + ), + ( + "nested_closure_indentation_normalized_issue159", + ground_truth_nested_closure_indentation_normalized_issue159, + idempotency_nested_closure_indentation_normalized_issue159 + ), + ( + "closure_argument_pipe_spacing_normalized_issue160", + ground_truth_closure_argument_pipe_spacing_normalized_issue160, + idempotency_closure_argument_pipe_spacing_normalized_issue160 + ), );