Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "nufmt"
version = "0.1.1"
version = "0.1.2"
edition = "2021"
authors = ["The NuShell Contributors"]
license = "MIT"
Expand Down
38 changes: 34 additions & 4 deletions src/formatting/calls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,19 @@ impl<'a> Formatter<'a> {
return;
}

// Write command name
// Write command name, normalizing multi-word command spacing
if call.head.end != 0 {
self.write_span(call.head);
if let Some(ref head) = head_text {
// For multi-word commands, normalize spacing between words
if head.contains(' ') && head.split_whitespace().count() > 1 {
let normalized = head.split_whitespace().collect::<Vec<_>>().join(" ");
self.write(&normalized);
} else {
self.write_span(call.head);
}
} else {
self.write_span(call.head);
}
}

if matches!(cmd_type, CommandType::Let) {
Expand Down Expand Up @@ -167,9 +177,21 @@ impl<'a> Formatter<'a> {
.filter(|arg| self.argument_belongs_to_call_source(call, arg))
.collect();

let head_text = self.call_head_text(call);

self.write("(");
if call.head.end != 0 {
self.write_span(call.head);
// Normalize multi-word command spacing
if let Some(ref head) = head_text {
if head.contains(' ') && head.split_whitespace().count() > 1 {
let normalized = head.split_whitespace().collect::<Vec<_>>().join(" ");
self.write(&normalized);
} else {
self.write_span(call.head);
}
} else {
self.write_span(call.head);
}
}
self.newline();
self.indent_level += 1;
Expand Down Expand Up @@ -1154,7 +1176,15 @@ impl<'a> Formatter<'a> {
self.write(&rendered);
}
}
other => self.write(&other.to_string()),
other => {
let rendered = other.to_string();
if rendered.contains("closure()") {
let normalized = rendered.replace("closure()", "closure");
self.write(&normalized);
} else {
self.write(&rendered);
}
}
}
}
}
74 changes: 72 additions & 2 deletions src/formatting/collections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,24 @@ impl<'a> Formatter<'a> {
.map(|(pattern, expr)| self.render_match_arm_lhs(pattern, expr))
.collect();

let should_align = self.should_preserve_match_arm_alignment(matches)
// Check if source has alignment preserved
let preserve_alignment = self.should_preserve_match_arm_alignment(matches)
&& rendered_lhs.iter().all(|lhs| !lhs.contains(&b'\n'));

// Additionally check if all RHS expressions will format as single-line
// to ensure idempotency - don't align if any RHS becomes multiline
let all_rhs_single_line = if preserve_alignment {
matches.iter().all(|(_, expr)| {
let rendered = self.probe_format(|probe| {
probe.format_block_or_expr(expr);
});
!rendered.contains(&b'\n')
})
} else {
false
};

let should_align = preserve_alignment && all_rhs_single_line;
let max_lhs_len = if should_align {
rendered_lhs.iter().map(Vec::len).max().unwrap_or(0)
} else {
Expand All @@ -408,7 +424,11 @@ impl<'a> Formatter<'a> {
}

self.write("=> ");
self.format_block_or_expr(expr);
if self.should_force_multiline_match_arm_block(expr) {
self.format_match_arm_block_multiline(expr);
} else {
self.format_block_or_expr(expr);
}
self.newline();
}

Expand All @@ -417,6 +437,56 @@ impl<'a> Formatter<'a> {
self.write("}");
}

/// Return `true` when a simple braced match-arm block would become
/// multiline once formatted, so we can emit multiline output in one pass.
fn should_force_multiline_match_arm_block(&self, expr: &Expression) -> bool {
let Expr::Block(block_id) = &expr.expr else {
return false;
};

let block = self.working_set.get_block(*block_id);
let source_has_newline = expr.span.end > expr.span.start
&& self.source[expr.span.start..expr.span.end].contains(&b'\n');

let is_simple = block.pipelines.len() == 1
&& block.pipelines[0].elements.len() == 1
&& !self.block_has_nested_structures(block)
&& !source_has_newline;

if !is_simple {
return false;
}

self.probe_format(|probe| probe.format_pipeline(&block.pipelines[0]))
.contains(&b'\n')
}

/// Format a match-arm block body as multiline, preserving the same
/// conditional-depth reset semantics as regular block expressions.
fn format_match_arm_block_multiline(&mut self, expr: &Expression) {
let Expr::Block(block_id) = &expr.expr else {
self.format_block_or_expr(expr);
return;
};

let block = self.working_set.get_block(*block_id);

self.write("{");

let saved_conditional_depth = self.conditional_context_depth;
self.conditional_context_depth = 0;

self.newline();
self.indent_level += 1;
self.format_block(block);
self.newline();
self.indent_level -= 1;
self.write_indent();
self.write("}");

self.conditional_context_depth = saved_conditional_depth;
}

/// Format a match pattern (value, variable, list, record, or, etc.).
pub(super) fn format_match_pattern(&mut self, pattern: &MatchPattern) {
match &pattern.pattern {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
error make {msg: "hello"}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
def par [jobs: list<closure>] { }
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
def foo [--editor: string] {
match $editor {
_ => {
error make {
msg: "unsupported editor"
label: {
span: (metadata $editor).span
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
error make {msg: "hello"}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
def par [jobs: list<closure>] { }
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
def foo [--editor: string] {
match $editor {
_ => {error make {msg: "unsupported editor", label: {span: (metadata $editor).span}}}
}
}
15 changes: 15 additions & 0 deletions tests/ground_truth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -684,4 +684,19 @@ fixture_tests!(
ground_truth_stderr_pipeline_redirection_pipe_not_duplicated_issue181,
idempotency_stderr_pipeline_redirection_pipe_not_duplicated_issue181
),
(
"list_closure_type_annotation_issue187",
ground_truth_list_closure_type_annotation_issue187,
idempotency_list_closure_type_annotation_issue187
),
(
"extraneous_spaces_in_multi_word_function_stripped_issue188",
ground_truth_extraneous_spaces_in_multi_word_function_stripped_issue188,
idempotency_extraneous_spaces_in_multi_word_function_stripped_issue188
),
(
"match_arm_formatting_is_idempotent_issue189",
ground_truth_match_arm_formatting_is_idempotent_issue189,
idempotency_match_arm_formatting_is_idempotent_issue189
),
);
68 changes: 50 additions & 18 deletions tests/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,30 @@ const VALID: &str = "# beginning of script comment
let one = 1
";

fn run_stdin(input: &str) -> std::process::Output {
let mut child = Command::new(get_test_binary())
/// Filter parser diagnostic noise from stderr that appears when `RUST_LOG` is set.
/// Removes lines that contain parser diagnostic messages from the `nu_parser` module.
fn filter_parser_diagnostics(stderr: &str) -> String {
stderr
.lines()
.filter(|line| !line.contains("nu_parser::"))
.collect::<Vec<_>>()
.join("\n")
}

/// Run `nufmt --stdin` with optional environment overrides.
fn run_stdin_with_env(input: &str, env: &[(&str, &str)]) -> std::process::Output {
let mut command = Command::new(get_test_binary());
command
.arg("--stdin")
.stdin(std::process::Stdio::piped())
.stdout(std::process::Stdio::piped())
.stderr(std::process::Stdio::piped())
.spawn()
.expect("Failed to spawn nufmt");
.stderr(std::process::Stdio::piped());

for (key, value) in env {
command.env(key, value);
}

let mut child = command.spawn().expect("Failed to spawn nufmt");

child
.stdin
Expand All @@ -29,6 +45,28 @@ fn run_stdin(input: &str) -> std::process::Output {
child.wait_with_output().expect("Failed to wait for nufmt")
}

/// Run `nufmt --stdin` with default environment.
fn run_stdin(input: &str) -> std::process::Output {
run_stdin_with_env(input, &[])
}

/// Run `nufmt --stdin` with `RUST_LOG=error` enabled.
fn run_stdin_with_rust_log_error(input: &str) -> std::process::Output {
run_stdin_with_env(input, &[("RUST_LOG", "error")])
}

/// Assert that a formatter run completed successfully without parser noise.
fn assert_no_parser_error_noise(output: &std::process::Output) {
let stderr = String::from_utf8_lossy(&output.stderr);
let cleaned_stderr = filter_parser_diagnostics(&stderr);

assert_eq!(output.status.code(), Some(0));
assert!(
!cleaned_stderr.contains("compile_block_with_id called with parse errors"),
"unexpected parser error noise on stderr: {cleaned_stderr}"
);
}

#[test]
fn failure_with_invalid_config() {
let dir = tempdir().unwrap();
Expand Down Expand Up @@ -315,25 +353,19 @@ fn format_fixtures_basic() {
#[test]
fn mixed_use_and_def_does_not_emit_parser_errors_issue136() {
let output = run_stdin("use a.nu\ndef abc [] { }\ndef xyz [] { }\n");
let stderr = String::from_utf8_lossy(&output.stderr);

assert_eq!(output.status.code(), Some(0));
assert!(
!stderr.contains("compile_block_with_id called with parse errors"),
"unexpected parser error noise on stderr: {stderr}"
);
assert_no_parser_error_noise(&output);
}

#[test]
fn cell_path_in_def_block_does_not_emit_parser_errors_issue141() {
let output = run_stdin("def main [] {\n$var.state\n}\n");
let stderr = String::from_utf8_lossy(&output.stderr);
assert_no_parser_error_noise(&output);
}

assert_eq!(output.status.code(), Some(0));
assert!(
!stderr.contains("compile_block_with_id called with parse errors"),
"unexpected parser error noise on stderr: {stderr}"
);
#[test]
fn cli_tests_do_not_fail_when_rust_log_error_is_set_issue190() {
let output = run_stdin_with_rust_log_error("use a.nu\ndef abc [] { }\ndef xyz [] { }\n");
assert_no_parser_error_noise(&output);
}

#[test]
Expand Down
Loading