feat(blend): add check and format commands - #25
Conversation
Reviewer's GuideAdds first-class Nickel-based Sequence diagram for blend format --check command flowsequenceDiagram
actor User
participant BlendCli as main
participant CmdFormat as cmd_format
participant Nickel as format_source
participant FS as FileSystem
participant Gen as generated
User->>BlendCli: Commands::Format
BlendCli->>CmdFormat: cmd_format(ctx, orders, check)
CmdFormat->>Gen: assert_orders_ready(orders_dir)
Gen-->>CmdFormat: Result
CmdFormat->>CmdFormat: selected_orders(ctx, orders)
loop each order_name
CmdFormat->>FS: read_to_string(order.ncl)
FS-->>CmdFormat: source
CmdFormat->>Nickel: format_source(source)
Nickel-->>CmdFormat: formatted
alt formatted == source
CmdFormat->>User: println("✓ path")
else formatted != source
alt check || ctx.dry_run
CmdFormat->>User: println("would format path")
else
CmdFormat->>FS: write(path, formatted)
FS-->>CmdFormat: Result
end
end
end
CmdFormat->>User: log::success(...) / log::info(...)
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Both
cmd_checkandcmd_formatdefine an identicalselected_ordershelper; consider pulling this into a shared function (e.g., incommands::helpers) to avoid duplication and keep order-selection behavior consistent in one place.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Both `cmd_check` and `cmd_format` define an identical `selected_orders` helper; consider pulling this into a shared function (e.g., in `commands::helpers`) to avoid duplication and keep order-selection behavior consistent in one place.
## Individual Comments
### Comment 1
<location path="blend/tests/sync_e2e.rs" line_range="150" />
<code_context>
+#[test]
+fn test_check_order_success() {
</code_context>
<issue_to_address>
**suggestion (testing):** Add an e2e test for `blend check` with a non-existent order name
Current tests cover the success path and the `from_file` missing case, but not when a user passes an unknown order name (handled in `cmd_check` as `Order '{order_name}' not found`). Please add an e2e test (e.g., `test_check_order_fails_when_order_missing`) that runs `blend --sandbox never check some-nonexistent-order`, asserts a non-zero exit code, and verifies that stderr includes the "Order '…' not found" message.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -149,6 +149,109 @@ fn test_sandbox_force_exec_probe() { | |||
| ); | |||
| } | |||
There was a problem hiding this comment.
suggestion (testing): Add an e2e test for blend check with a non-existent order name
Current tests cover the success path and the from_file missing case, but not when a user passes an unknown order name (handled in cmd_check as Order '{order_name}' not found). Please add an e2e test (e.g., test_check_order_fails_when_order_missing) that runs blend --sandbox never check some-nonexistent-order, asserts a non-zero exit code, and verifies that stderr includes the "Order '…' not found" message.
57b9db4 to
90005ef
Compare
90005ef to
beb5c24
Compare
Summary
blend checkto evaluate/typecheck selected or allorder.nclfiles through the embedded Nickel evaluatorblend check, including missing activefrom_filesourcesblend format/blend fmtwith--checkand dry-run support using the in-process Nickel formatterblend checkplusblend format --checkstyle: format ordersjust check, and cover the new commands in E2E testsCloses #22.
Verification
actionlint .github/workflows/blend-ci.yml .github/workflows/orders-ci.ymlcargo fmt -p blend --checkcargo test -p blend test_check_ordercargo test -p blend test_formatcargo test -p blendHOME=/private/tmp/blend-ci-home-split-style cargo run -p blend -- checkHOME=/private/tmp/blend-ci-home-split-style cargo run -p blend -- format --check