feat(blend): add check and format commands - #24
Closed
frantic1048 wants to merge 2 commits into
Closed
Conversation
Reviewer's GuideAdds new Nickel-based Sequence diagram for new blend check and format commandssequenceDiagram
actor User
participant Main as main_rs
participant Ctx as Context
participant CmdCheck as cmd_check
participant CmdFormat as cmd_format
participant Eval as NickelEvaluator
participant Fmt as format_source
User->>Main: blend check [orders]
Main->>Ctx: Context::new(cli)
Main->>CmdCheck: cmd_check(ctx, orders)
CmdCheck->>Eval: NickelEvaluator::new(metadata)
loop each selected order
CmdCheck->>Eval: evaluate(order_ncl_path)
Eval-->>CmdCheck: Result
end
CmdCheck-->>User: summary / exit status
User->>Main: blend format [orders] [--check]
Main->>Ctx: Context::new(cli)
Main->>CmdFormat: cmd_format(ctx, orders, check)
loop each selected order
CmdFormat->>Fmt: format_source(source)
Fmt-->>CmdFormat: formatted_source
alt check or ctx.dry_run
CmdFormat-->>CmdFormat: [no write]
else write changes
CmdFormat->>FileSystem: write(order_ncl_path, formatted_source)
end
end
CmdFormat-->>User: summary / exit status
Flow diagram for resolving and persisting blend_dir stateflowchart TD
A[Context::new] --> B[home_dir_from_cli]
B --> C[StateStore::from_env_for_home]
C --> D[resolve_blend_dir]
D --> E[find_blend_dir_from_state_or_config]
E --> F{state.read_blend_dir}
F -->|Some path| G[use state blend_dir]
F -->|None| H[find_blend_dir_from_config]
H -->|Some path| I[use config blend_dir]
H -->|None| J[bail: no blend_dir]
subgraph On_successful_command
K[Context::write_blend_dir_state]
K --> L[state.write_blend_dir]
L --> M[state.json under XDG_STATE_HOME]
end
G --> On_successful_command
I --> On_successful_command
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Both
checkandformatdefine an identicalselected_ordershelper; consider extracting this into a shared function (e.g., in a small internal module) to avoid duplication and keep selection semantics in one place. - The temp-file-then-rename logic in
StateStore::writeandStateStore::write_blend_diris nearly identical; factoring this into a shared helper would reduce repetition and make future changes to the write pattern less error-prone.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Both `check` and `format` define an identical `selected_orders` helper; consider extracting this into a shared function (e.g., in a small internal module) to avoid duplication and keep selection semantics in one place.
- The temp-file-then-rename logic in `StateStore::write` and `StateStore::write_blend_dir` is nearly identical; factoring this into a shared helper would reduce repetition and make future changes to the write pattern less error-prone.
## Individual Comments
### Comment 1
<location path="blend/src/commands/format.rs" line_range="8-15" />
<code_context>
+use crate::nickel::{NickelEvaluator, generated};
+use crate::output::log;
+
+fn selected_orders(ctx: &Context, orders: &[String]) -> Vec<String> {
+ let mut selected: Vec<String> = if orders.is_empty() {
+ discover_orders(&ctx.orders_dir).into_iter().collect()
+ } else {
+ orders.to_vec()
+ };
+ selected.sort();
+ selected
+}
+
</code_context>
<issue_to_address>
**suggestion:** Deduplicate `selected_orders` helper shared by `check` and `format` commands
This helper is identical to the one in `commands/check.rs`. To avoid duplication and keep order-selection behavior (sorting, defaults, etc.) consistent, please extract it into a shared helper (e.g., `commands::helpers` or a small shared module) and reuse it in both commands.
Suggested implementation:
```rust
use console::style;
use crate::context::Context;
use crate::nickel::{format_source, generated};
use crate::output::log;
use crate::commands::helpers::selected_orders;
```
1. Introduce a shared helper module, for example `blend/src/commands/helpers.rs`, with:
```rust
use crate::compose::discover_orders;
use crate::context::Context;
pub fn selected_orders(ctx: &Context, orders: &[String]) -> Vec<String> {
let mut selected: Vec<String> = if orders.is_empty() {
discover_orders(&ctx.orders_dir).into_iter().collect()
} else {
orders.to_vec()
};
selected.sort();
selected
}
```
2. In `blend/src/commands/mod.rs` (or the relevant module file), `pub mod helpers;` so `crate::commands::helpers::selected_orders` resolves.
3. In `blend/src/commands/check.rs`, remove the local `selected_orders` definition and import/use `crate::commands::helpers::selected_orders` in the same way as `format.rs`.
4. Ensure any existing calls in `format.rs` and `check.rs` continue to call `selected_orders(ctx, orders)` without change; only the definition location and imports should move.
</issue_to_address>
### Comment 2
<location path="blend/tests/sync_e2e.rs" line_range="152-161" />
<code_context>
}
}
+ #[test]
+ fn from_env_for_home_falls_back_to_supplied_home() {
+ let prev_xdg = std::env::var_os("XDG_STATE_HOME");
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen `test_blend_order_rejects_stale_blend_dir_config_field` by asserting a non-zero exit status.
The test already verifies that a stale `blend_dir` surfaces a Nickel contract error on stdout/stderr. To also prove the CLI actually fails, please assert that the process exits with a non-success status, e.g.:
```rust
assert!(
!output.status.success(),
"blend view with stale blend_dir should fail\nstdout: {stdout}\nstderr: {stderr}",
);
```
This ensures contract violations affect the exit code, which is important for scripting and automation.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+8
to
+15
| fn selected_orders(ctx: &Context, orders: &[String]) -> Vec<String> { | ||
| let mut selected: Vec<String> = if orders.is_empty() { | ||
| discover_orders(&ctx.orders_dir).into_iter().collect() | ||
| } else { | ||
| orders.to_vec() | ||
| }; | ||
| selected.sort(); | ||
| selected |
There was a problem hiding this comment.
suggestion: Deduplicate selected_orders helper shared by check and format commands
This helper is identical to the one in commands/check.rs. To avoid duplication and keep order-selection behavior (sorting, defaults, etc.) consistent, please extract it into a shared helper (e.g., commands::helpers or a small shared module) and reuse it in both commands.
Suggested implementation:
use console::style;
use crate::context::Context;
use crate::nickel::{format_source, generated};
use crate::output::log;
use crate::commands::helpers::selected_orders;- Introduce a shared helper module, for example
blend/src/commands/helpers.rs, with:use crate::compose::discover_orders; use crate::context::Context; pub fn selected_orders(ctx: &Context, orders: &[String]) -> Vec<String> { let mut selected: Vec<String> = if orders.is_empty() { discover_orders(&ctx.orders_dir).into_iter().collect() } else { orders.to_vec() }; selected.sort(); selected }
- In
blend/src/commands/mod.rs(or the relevant module file),pub mod helpers;socrate::commands::helpers::selected_ordersresolves. - In
blend/src/commands/check.rs, remove the localselected_ordersdefinition and import/usecrate::commands::helpers::selected_ordersin the same way asformat.rs. - Ensure any existing calls in
format.rsandcheck.rscontinue to callselected_orders(ctx, orders)without change; only the definition location and imports should move.
Comment on lines
+152
to
+161
| #[test] | ||
| fn test_check_order_success() { | ||
| let home = TempDir::new().unwrap(); | ||
| let orders = fixtures_dir(); | ||
|
|
||
| let output = run_blend( | ||
| home.path(), | ||
| &orders, | ||
| &["--sandbox", "never", "check", "toml-basic"], | ||
| ); |
There was a problem hiding this comment.
suggestion (testing): Strengthen test_blend_order_rejects_stale_blend_dir_config_field by asserting a non-zero exit status.
The test already verifies that a stale blend_dir surfaces a Nickel contract error on stdout/stderr. To also prove the CLI actually fails, please assert that the process exits with a non-success status, e.g.:
assert!(
!output.status.success(),
"blend view with stale blend_dir should fail\nstdout: {stdout}\nstderr: {stderr}",
);This ensures contract violations affect the exit code, which is important for scripting and automation.
frantic1048
force-pushed
the
codex/blend-check-format
branch
6 times, most recently
from
June 11, 2026 16:33
af89783 to
f1926ee
Compare
frantic1048
changed the title
frantic1048
force-pushed
the
codex/blend-check-format
branch
from
June 11, 2026 16:37
f1926ee to
381a27f
Compare
frantic1048
force-pushed
the
codex/blend-check-format
branch
from
June 11, 2026 16:40
381a27f to
2fde8e9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 --checkjust 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 blendHOME=/private/tmp/blend-ci-home-semantics cargo run -p blend -- checkHOME=/private/tmp/blend-ci-home-semantics cargo run -p blend -- format --check