From 0c6bbe79b8c6027f89b2ce509f40c1f6977f26ec Mon Sep 17 00:00:00 2001 From: Ville Kopio Date: Fri, 27 Feb 2026 10:17:11 +0200 Subject: [PATCH 1/9] Update flake so that the project compiles The Rust version was too old for some of the dependencies. --- flake.lock | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/flake.lock b/flake.lock index 2b07bb3..a1796cc 100644 --- a/flake.lock +++ b/flake.lock @@ -20,11 +20,11 @@ }, "nixpkgs": { "locked": { - "lastModified": 1747426788, - "narHash": "sha256-N4cp0asTsJCnRMFZ/k19V9akkxb7J/opG+K+jU57JGc=", + "lastModified": 1772082373, + "narHash": "sha256-wySf8a6hvuqgFdwvvzPPTARBCMLDz7WFAufGkllD1M4=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "12a55407652e04dcf2309436eb06fef0d3713ef3", + "rev": "26eaeac4e409d7b5a6bf6f90a2a2dc223c78d915", "type": "github" }, "original": { @@ -49,11 +49,11 @@ ] }, "locked": { - "lastModified": 1747449297, - "narHash": "sha256-veyXchTz6eWwvuW5X49UluHkheHkFcqHJSwGuKBhrmQ=", + "lastModified": 1772161521, + "narHash": "sha256-HmbcapTlcRqtryLUaJhH8t1mz6DaSJT+nxvWIl2bIPU=", "owner": "oxalica", "repo": "rust-overlay", - "rev": "f44db7d7cea4528288780c6347756173a8248225", + "rev": "9b2965450437541d25fde167d8bebfd01c156cef", "type": "github" }, "original": { @@ -84,11 +84,11 @@ ] }, "locked": { - "lastModified": 1747469671, - "narHash": "sha256-bo1ptiFoNqm6m1B2iAhJmWCBmqveLVvxom6xKmtuzjg=", + "lastModified": 1770228511, + "narHash": "sha256-wQ6NJSuFqAEmIg2VMnLdCnUc0b7vslUohqqGGD+Fyxk=", "owner": "numtide", "repo": "treefmt-nix", - "rev": "ab0378b61b0d85e73a8ab05d5c6029b5bd58c9fb", + "rev": "337a4fe074be1042a35086f15481d763b8ddc0e7", "type": "github" }, "original": { From 69b07c93000db187198063cbe336884e436f9eba Mon Sep 17 00:00:00 2001 From: Ville Kopio Date: Fri, 27 Feb 2026 10:21:14 +0200 Subject: [PATCH 2/9] Test that cell-path works as a function parameter --- tests/fixtures/expected/cell_path.nu | 2 ++ tests/fixtures/input/cell_path.nu | 3 +++ 2 files changed, 5 insertions(+) diff --git a/tests/fixtures/expected/cell_path.nu b/tests/fixtures/expected/cell_path.nu index e6db170..82717af 100644 --- a/tests/fixtures/expected/cell_path.nu +++ b/tests/fixtures/expected/cell_path.nu @@ -4,3 +4,5 @@ $list.0 $list.5 $data.users.0.name $table.column.0 +foo bar.baz +def foo [key: cell-path] { } diff --git a/tests/fixtures/input/cell_path.nu b/tests/fixtures/input/cell_path.nu index 5f532e6..33bff75 100644 --- a/tests/fixtures/input/cell_path.nu +++ b/tests/fixtures/input/cell_path.nu @@ -4,3 +4,6 @@ $list.5 $data.users.0.name $table.column.0 + foo bar.baz +def foo [key: cell-path] { +} From 4254a498188ac0962b8e1ddcdeacb8f979af97a5 Mon Sep 17 00:00:00 2001 From: Ville Kopio Date: Fri, 27 Feb 2026 10:22:01 +0200 Subject: [PATCH 3/9] Test a function with a closure and a try-catch --- tests/fixtures/expected/def_statement.nu | 9 +++++++++ tests/fixtures/input/def_statement.nu | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/tests/fixtures/expected/def_statement.nu b/tests/fixtures/expected/def_statement.nu index fb896eb..8689d3e 100644 --- a/tests/fixtures/expected/def_statement.nu +++ b/tests/fixtures/expected/def_statement.nu @@ -12,3 +12,12 @@ def complex [ --flag(-f) --value(-v): int = 5 ] { print $"($a) ($b) $flag $value" } +def test [body: closure] { + try { + do $body + true + } catch { |err| + print $err.rendered + false + } +} diff --git a/tests/fixtures/input/def_statement.nu b/tests/fixtures/input/def_statement.nu index 6b3ebcb..9e042a2 100644 --- a/tests/fixtures/input/def_statement.nu +++ b/tests/fixtures/input/def_statement.nu @@ -12,3 +12,12 @@ def complex [ --flag(-f) --value(-v): int = 5 ] { print $"($a) ($b) $flag $value" } +def test [body: closure] { + try { + do $body + true + } catch { |err| + print $err.rendered + false + } +} From 445a161a898f2404efc1a5c5e0f6f590fbc95570 Mon Sep 17 00:00:00 2001 From: Ville Kopio Date: Sun, 1 Mar 2026 14:29:33 +0200 Subject: [PATCH 4/9] Extract cell path literal test to their own file --- tests/fixtures/expected/cell_path.nu | 6 ------ tests/fixtures/expected/cell_path_literals.nu | 6 ++++++ tests/fixtures/input/cell_path.nu | 6 ------ tests/fixtures/input/cell_path_literals.nu | 6 ++++++ tests/ground_truth.rs | 12 ++++++++++++ tests/run_ground_truth_tests.nu | 1 + 6 files changed, 25 insertions(+), 12 deletions(-) create mode 100644 tests/fixtures/expected/cell_path_literals.nu create mode 100644 tests/fixtures/input/cell_path_literals.nu diff --git a/tests/fixtures/expected/cell_path.nu b/tests/fixtures/expected/cell_path.nu index 82717af..3d138c5 100644 --- a/tests/fixtures/expected/cell_path.nu +++ b/tests/fixtures/expected/cell_path.nu @@ -1,8 +1,2 @@ -$record.name -$record.a.b.c -$list.0 -$list.5 -$data.users.0.name -$table.column.0 foo bar.baz def foo [key: cell-path] { } diff --git a/tests/fixtures/expected/cell_path_literals.nu b/tests/fixtures/expected/cell_path_literals.nu new file mode 100644 index 0000000..e6db170 --- /dev/null +++ b/tests/fixtures/expected/cell_path_literals.nu @@ -0,0 +1,6 @@ +$record.name +$record.a.b.c +$list.0 +$list.5 +$data.users.0.name +$table.column.0 diff --git a/tests/fixtures/input/cell_path.nu b/tests/fixtures/input/cell_path.nu index 33bff75..fe91835 100644 --- a/tests/fixtures/input/cell_path.nu +++ b/tests/fixtures/input/cell_path.nu @@ -1,9 +1,3 @@ - $record.name - $record.a.b.c - $list.0 - $list.5 - $data.users.0.name - $table.column.0 foo bar.baz def foo [key: cell-path] { } diff --git a/tests/fixtures/input/cell_path_literals.nu b/tests/fixtures/input/cell_path_literals.nu new file mode 100644 index 0000000..5f532e6 --- /dev/null +++ b/tests/fixtures/input/cell_path_literals.nu @@ -0,0 +1,6 @@ + $record.name + $record.a.b.c + $list.0 + $list.5 + $data.users.0.name + $table.column.0 diff --git a/tests/ground_truth.rs b/tests/ground_truth.rs index 9e4d8bb..39d59b0 100644 --- a/tests/ground_truth.rs +++ b/tests/ground_truth.rs @@ -306,6 +306,12 @@ fn ground_truth_range() { run_ground_truth_test(&test_binary, "range"); } +#[test] +fn ground_truth_cell_path_literals() { + let test_binary = get_test_binary(); + run_ground_truth_test(&test_binary, "cell_path_literals"); +} + #[test] fn ground_truth_cell_path() { let test_binary = get_test_binary(); @@ -594,6 +600,12 @@ fn idempotency_range() { run_idempotency_test(&test_binary, "range"); } +#[test] +fn idempotency_cell_path_literals() { + let test_binary = get_test_binary(); + run_idempotency_test(&test_binary, "cell_path_literals"); +} + #[test] fn idempotency_cell_path() { let test_binary = get_test_binary(); diff --git a/tests/run_ground_truth_tests.nu b/tests/run_ground_truth_tests.nu index 12a323d..da7744c 100755 --- a/tests/run_ground_truth_tests.nu +++ b/tests/run_ground_truth_tests.nu @@ -28,6 +28,7 @@ const TEST_CONSTRUCTS = { "subexpression" "binary_ops" "range" + "cell_path_literals" "cell_path" "spread" ] From a8d008f578deb40085a23871b0ba68ec94370be2 Mon Sep 17 00:00:00 2001 From: Ville Kopio Date: Sun, 1 Mar 2026 15:33:34 +0200 Subject: [PATCH 5/9] Fix CellPath first element formatting As the CellPath and FullCellPath shared the formatting logic, CellPaths got a dot incorrectly prepended to them. Now their formatting is separated to take this into account. --- src/formatting.rs | 55 +++++++++++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/src/formatting.rs b/src/formatting.rs index 9ffcad4..a297f2f 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -8,8 +8,9 @@ use log::{debug, trace}; use nu_parser::parse; use nu_protocol::{ ast::{ - Argument, Block, Expr, Expression, ExternalArgument, ListItem, MatchPattern, PathMember, - Pattern, Pipeline, PipelineElement, PipelineRedirection, RecordItem, RedirectionTarget, + Argument, Block, CellPath, Expr, Expression, ExternalArgument, FullCellPath, ListItem, + MatchPattern, PathMember, Pattern, Pipeline, PipelineElement, PipelineRedirection, + RecordItem, RedirectionTarget, }, engine::{EngineState, StateWorkingSet}, Signature, Span, SyntaxShape, @@ -342,10 +343,9 @@ impl<'a> Formatter<'a> { Expr::Table(table) => self.format_table(&table.columns, &table.rows), Expr::Range(range) => self.format_range(range), - Expr::CellPath(cell_path) => self.format_cell_path_members(&cell_path.members), + Expr::CellPath(cell_path) => self.format_cell_path(cell_path), Expr::FullCellPath(full_path) => { - self.format_expression(&full_path.head); - self.format_cell_path_members(&full_path.tail); + self.format_full_cell_path(full_path); } Expr::RowCondition(block_id) => { @@ -683,23 +683,40 @@ impl<'a> Formatter<'a> { self.write("]"); } - /// Format cell path members (shared between `CellPath` and `FullCellPath`) - fn format_cell_path_members(&mut self, members: &[PathMember]) { - for member in members { + /// Format a `CellPath` + fn format_cell_path(&mut self, cell_path: &CellPath) { + for (i, member) in cell_path.members.iter().enumerate() { + if i > 0 { + self.write("."); + } + self.format_cell_path_member(member); + } + } + + /// Format a `FullCellPath` + fn format_full_cell_path(&mut self, cell_path: &FullCellPath) { + self.format_expression(&cell_path.head); + + for member in &cell_path.tail { self.write("."); - match member { - PathMember::String { val, optional, .. } => { - if *optional { - self.write("?"); - } - self.write(val); + self.format_cell_path_member(member); + } + } + + /// Format cell path member (shared between `CellPath` and `FullCellPath`) + fn format_cell_path_member(&mut self, member: &PathMember) { + match member { + PathMember::String { val, optional, .. } => { + if *optional { + self.write("?"); } - PathMember::Int { val, optional, .. } => { - if *optional { - self.write("?"); - } - self.write(&val.to_string()); + self.write(val); + } + PathMember::Int { val, optional, .. } => { + if *optional { + self.write("?"); } + self.write(&val.to_string()); } } } From d3ff7ab84372f657b52a506c9b3e620fc6413bee Mon Sep 17 00:00:00 2001 From: Ville Kopio Date: Sun, 1 Mar 2026 15:53:20 +0200 Subject: [PATCH 6/9] Fix closure type hints This prevents the type hints from being formatted as `closure()`. --- src/formatting.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/formatting.rs b/src/formatting.rs index a297f2f..f99a89a 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -609,7 +609,12 @@ impl<'a> Formatter<'a> { self.write(¶m.name); if param.shape != SyntaxShape::Any { self.write(": "); - self.write(&format!("{}", param.shape)); + match ¶m.shape { + // Fixes an issue in which closure type hints were formatted as `closure()`. + // TODO: This feels hacky. Should this be addressed in the `nu-protocol` crate instead? + SyntaxShape::Closure(Option::None) => self.write("closure"), + _ => self.write(&format!("{}", param.shape)), + } } } From f4f228a720d334c456423a1d1039e6bf4600b403 Mon Sep 17 00:00:00 2001 From: Ville Kopio Date: Sun, 1 Mar 2026 17:45:08 +0200 Subject: [PATCH 7/9] Ensure that closure params are not dropped --- src/formatting.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/formatting.rs b/src/formatting.rs index f99a89a..ffa073f 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -511,9 +511,12 @@ impl<'a> Formatter<'a> { /// Format an expression that could be a block or a regular expression fn format_block_or_expr(&mut self, expr: &Expression) { match &expr.expr { - Expr::Block(block_id) | Expr::Closure(block_id) => { + Expr::Block(block_id) => { self.format_block_expression(*block_id, expr.span, true); } + Expr::Closure(block_id) => { + self.format_closure_expression(*block_id, expr.span); + } _ => self.format_expression(expr), } } From 69ccacee96f5ca9e939d4e67e91f92bd85a0cf01 Mon Sep 17 00:00:00 2001 From: Ville Kopio Date: Sun, 1 Mar 2026 17:46:39 +0200 Subject: [PATCH 8/9] Make closure parameter formatting less error prone Ensures that `has_params` resolves to `true` regardless the amount of whitespace. The pipe char indexes are properly checked and the parameters are trimmed properly. Also, removes trailing whitespace from multi-line closures. --- src/formatting.rs | 61 ++++++++++++-------- tests/fixtures/expected/closure.nu | 4 ++ tests/fixtures/expected/def_statement.nu | 2 +- tests/fixtures/expected/nested_structures.nu | 2 +- tests/fixtures/input/closure.nu | 7 +++ tests/fixtures/input/def_statement.nu | 2 +- 6 files changed, 51 insertions(+), 27 deletions(-) diff --git a/src/formatting.rs b/src/formatting.rs index ffa073f..aa5a76c 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -845,43 +845,55 @@ impl<'a> Formatter<'a> { /// Format a closure expression fn format_closure_expression(&mut self, block_id: nu_protocol::BlockId, span: Span) { let content = self.get_span_content(span); - let has_params = content.starts_with(b"{|") || content.starts_with(b"{ |"); + let has_params = content + .iter() + .skip(1) // Skip '{' + .find(|b| !b.is_ascii_whitespace()) + .is_some_and(|char| char.eq(&b'|')); if !has_params { self.format_block_expression(block_id, span, true); return; } - // Find the end of the parameter section (second |) - let param_end = content.iter().position(|&b| b == b'|').and_then(|first| { - content[first + 1..] - .iter() - .position(|&b| b == b'|') - .map(|p| first + 1 + p + 1) - }); + // Find the start of the parameter section (first |) + let Some(first_pipe_index) = content.iter().position(|&b| b == b'|') else { + self.write_bytes(&content); + return; + }; - let Some(end) = param_end else { + // Find the end of the parameter section (second |) + let Some(second_pipe_index) = content[first_pipe_index + 1..] + .iter() + .position(|&b| b == b'|') + .map(|p| first_pipe_index + 1 + p) + else { self.write_bytes(&content); return; }; self.write("{|"); // Extract and trim parameter content - let params = &content[2..end - 1]; - let trimmed: Vec = params - .iter() - .copied() - .skip_while(|b| b.is_ascii_whitespace()) - .collect::>() - .into_iter() - .rev() - .skip_while(|b| b.is_ascii_whitespace()) - .collect::>() - .into_iter() - .rev() - .collect(); - self.write_bytes(&trimmed); - self.write("| "); + let params = &content[first_pipe_index + 1..second_pipe_index]; + 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(k), Some(v)) = (sub_parts.next(), sub_parts.next()) { + self.write_bytes(k.trim_ascii()); + self.write_bytes(b": "); + self.write_bytes(v.trim_ascii()); + } else { + self.write_bytes(param.trim_ascii()); + } + + if params_iter.peek().is_some() { + self.write_bytes(b", "); + } + } + + self.write("|"); let block = self.working_set.get_block(block_id); let is_simple = block.pipelines.len() == 1 @@ -889,6 +901,7 @@ impl<'a> Formatter<'a> { && !self.block_has_nested_structures(block); if is_simple { + self.space(); self.format_block(block); self.write(" }"); } else { diff --git a/tests/fixtures/expected/closure.nu b/tests/fixtures/expected/closure.nu index 29be762..4566130 100644 --- a/tests/fixtures/expected/closure.nu +++ b/tests/fixtures/expected/closure.nu @@ -1,8 +1,12 @@ {|| 1 } +{|| 1 } +{|x| $x } {|x| $x } {|x| $x * 2 } {|x| $x * 2 } {|x, y| $x + $y } +{|x, y| $x + $y } {|x: int| $x * 2 } {|x: int, y: int| $x + $y } +{|x: int, y: int| $x + $y } {|x| $x * 2 } diff --git a/tests/fixtures/expected/def_statement.nu b/tests/fixtures/expected/def_statement.nu index 8689d3e..3a05809 100644 --- a/tests/fixtures/expected/def_statement.nu +++ b/tests/fixtures/expected/def_statement.nu @@ -16,7 +16,7 @@ def test [body: closure] { try { do $body true - } catch { |err| + } catch {|err| print $err.rendered false } diff --git a/tests/fixtures/expected/nested_structures.nu b/tests/fixtures/expected/nested_structures.nu index 8711748..9e5df88 100644 --- a/tests/fixtures/expected/nested_structures.nu +++ b/tests/fixtures/expected/nested_structures.nu @@ -69,7 +69,7 @@ } } # Nested closures in data -let transform = {|data| +let transform = {|data| $data | each {|item| {|x| $x * $item } } } # Nested control flow diff --git a/tests/fixtures/input/closure.nu b/tests/fixtures/input/closure.nu index 4db61ab..7389cc4 100644 --- a/tests/fixtures/input/closure.nu +++ b/tests/fixtures/input/closure.nu @@ -1,8 +1,15 @@ {|| 1 } +{ || + 1 } {|x| $x } +{ |x| + $x } {|x| $x * 2 } {|x| $x * 2 } {|x, y| $x + $y } +{ | x, y | $x + $y } {|x: int| $x * 2 } {|x: int, y: int| $x + $y } +{|x: int , + y : int | $x + $y } {|x| $x * 2 } diff --git a/tests/fixtures/input/def_statement.nu b/tests/fixtures/input/def_statement.nu index 9e042a2..e365d22 100644 --- a/tests/fixtures/input/def_statement.nu +++ b/tests/fixtures/input/def_statement.nu @@ -16,7 +16,7 @@ def test [body: closure] { try { do $body true - } catch { |err| + } catch { |err| print $err.rendered false } From e05c98bc3b1d2da96e42c311016be7433384d8aa Mon Sep 17 00:00:00 2001 From: Ville Kopio Date: Sun, 1 Mar 2026 17:57:54 +0200 Subject: [PATCH 9/9] Improve naming --- src/formatting.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/formatting.rs b/src/formatting.rs index aa5a76c..3644a71 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -880,10 +880,10 @@ impl<'a> Formatter<'a> { while let Some(param) = params_iter.next() { let mut sub_parts = param.splitn(2, |&b| b == b':'); - if let (Some(k), Some(v)) = (sub_parts.next(), sub_parts.next()) { - self.write_bytes(k.trim_ascii()); + 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(v.trim_ascii()); + self.write_bytes(type_hint.trim_ascii()); } else { self.write_bytes(param.trim_ascii()); }