landing a better organized cli structure#6
Merged
Bechma merged 1 commit intocyberfabric:mainfrom Mar 2, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughConsolidates CLI argument types (renames CommonArgs → BuildRunArgs and removes flattened common_args fields), replaces the Setup command hierarchy with Mod and Tools variants, adds a new mod subcommand (Init, Add), updates build/run to use BuildRunArgs, and filters out "*sdk" library targets in module parsing. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/cli/src/common.rs`:
- Around line 95-97: Replace the stdout println! used when a local config module
is missing with stderr output to avoid polluting machine-readable stdout;
specifically change the println! call that prints "info: config module '{}' not
found locally, retrieving it from the registry" (the call referencing module.0)
to use eprintln! (or a structured logger writing to stderr) so the fallback
status message goes to stderr instead of stdout.
In `@crates/cli/src/mod/init.rs`:
- Around line 44-46: When self.local_path is Some we must not forward
self.subfolder (which defaults to "Init"); change the tuple in init.rs so the
second element is None instead of self.subfolder.clone() when entering local
template mode — i.e. replace (None, self.subfolder.clone(), None) with (None,
None, None) (or at least (None, None, None) for git, subfolder, branch) so local
templates don't get a forced subfolder; update any pattern/type expectations if
necessary to use Option<String>.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
crates/cli/src/build/mod.rscrates/cli/src/common.rscrates/cli/src/main.rscrates/cli/src/mod/add.rscrates/cli/src/mod/init.rscrates/cli/src/mod/mod.rscrates/cli/src/run/mod.rscrates/cli/src/setup/config.rscrates/cli/src/setup/mod.rscrates/cli/src/tools/mod.rscrates/module-parser/src/metadata.rs
💤 Files with no reviewable changes (2)
- crates/cli/src/setup/config.rs
- crates/cli/src/setup/mod.rs
Signed-off-by: Bechma <19294519+Bechma@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/cli/src/mod/init.rs (1)
44-55:⚠️ Potential issue | 🟠 MajorLocal template mode still carries a forced subfolder via
auto_path.At Line 51,
auto_pathis always populated fromsubfolder, so--local-pathstill inherits"Init"implicitly instead of being truly local-path driven.💡 Proposed fix
- let (git, branch) = if self.local_path.is_some() { + let local_mode = self.local_path.is_some(); + let (git, branch) = if local_mode { (None, None) } else { (self.git.clone(), self.branch.clone()) }; generate(GenerateArgs { template_path: TemplatePath { - auto_path: self.subfolder.clone(), + auto_path: if local_mode { None } else { self.subfolder.clone() }, git, path: self.local_path.clone(), subfolder: None, // This is only used when git, path and favorite are not specified branch,In cargo-generate, when `TemplatePath.path` is provided (local template mode), is `TemplatePath.auto_path` still applied to resolve the final template directory?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/cli/src/mod/init.rs` around lines 44 - 55, The template auto_path (TemplatePath.auto_path) is always set from self.subfolder causing local-path mode to still force a subfolder; update the GenerateArgs/TemplatePath construction so that when self.local_path.is_some() the auto_path is None (or only set when not using local_path), i.e. branch the value used for TemplatePath.auto_path based on self.local_path (or reuse the earlier git/branch conditional) so TemplatePath.auto_path is cleared in local template mode.
🧹 Nitpick comments (1)
crates/module-parser/src/metadata.rs (1)
15-15: Consider adding documentation and refining the "sdk" filter.A few observations on this new filter:
Missing rationale: There's no comment explaining why SDK targets should be excluded. Future maintainers will wonder about the intent.
Case sensitivity:
ends_with("sdk")is case-sensitive, so targets named"MySdk"or"MySDK"would not be filtered. Verify this is the intended behavior.Magic string: Consider extracting
"sdk"to a constant with a descriptive name.💡 Suggested improvement
- if t.is_lib() && !t.name.ends_with("sdk") { + // Skip SDK library targets as they are auxiliary crates not intended for module registration + if t.is_lib() && !t.name.ends_with("sdk") {Or, if case-insensitivity is desired:
- if t.is_lib() && !t.name.ends_with("sdk") { + // Skip SDK library targets as they are auxiliary crates not intended for module registration + if t.is_lib() && !t.name.to_lowercase().ends_with("sdk") {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/module-parser/src/metadata.rs` at line 15, Add a short comment above the filter in metadata.rs explaining why library targets with an SDK suffix are excluded, extract the literal "sdk" into a clearly named constant (e.g., SDK_SUFFIX) and use it in the predicate, and make the check case-insensitive (e.g., compare lowercase/uppercased versions of t.name or use a case-insensitive comparison) so names like "MySdk" or "MySDK" are also filtered; retain the use of t.is_lib() and t.name to locate the condition to change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/cli/src/mod/init.rs`:
- Around line 44-55: The template auto_path (TemplatePath.auto_path) is always
set from self.subfolder causing local-path mode to still force a subfolder;
update the GenerateArgs/TemplatePath construction so that when
self.local_path.is_some() the auto_path is None (or only set when not using
local_path), i.e. branch the value used for TemplatePath.auto_path based on
self.local_path (or reuse the earlier git/branch conditional) so
TemplatePath.auto_path is cleared in local template mode.
---
Nitpick comments:
In `@crates/module-parser/src/metadata.rs`:
- Line 15: Add a short comment above the filter in metadata.rs explaining why
library targets with an SDK suffix are excluded, extract the literal "sdk" into
a clearly named constant (e.g., SDK_SUFFIX) and use it in the predicate, and
make the check case-insensitive (e.g., compare lowercase/uppercased versions of
t.name or use a case-insensitive comparison) so names like "MySdk" or "MySDK"
are also filtered; retain the use of t.is_lib() and t.name to locate the
condition to change.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
crates/cli/src/build/mod.rscrates/cli/src/common.rscrates/cli/src/main.rscrates/cli/src/mod/add.rscrates/cli/src/mod/init.rscrates/cli/src/mod/mod.rscrates/cli/src/run/mod.rscrates/cli/src/setup/config.rscrates/cli/src/setup/mod.rscrates/cli/src/tools/mod.rscrates/module-parser/src/metadata.rs
💤 Files with no reviewable changes (2)
- crates/cli/src/setup/mod.rs
- crates/cli/src/setup/config.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/cli/src/common.rs
This was referenced Mar 4, 2026
This was referenced Mar 11, 2026
Merged
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 by CodeRabbit
New Features
Bug Fixes
Improvements