Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the help command output by reorganizing commands into two clear categories ("Vite+ Commands" and "Package Manager Commands") and reordering them by importance/usage frequency. The PR also adds a top-level info command that forwards to the package manager's view/info functionality.
Changes:
- Restructured help output with categorized commands and improved formatting
- Added
infocommand as a top-level alias forvite pm view - Updated command descriptions for consistency and clarity
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/global/binding/src/lib.rs | Changed argument parsing to use FromArgMatches for custom help handling and added normalize_help_args function to convert "help" subcommands to "--help" flags |
| packages/global/binding/src/cli.rs | Added Info command, created command_with_help() function with custom help template, and updated command descriptions |
| packages/global/binding/src/commands/pm.rs | Updated description comment to match the CLI change |
| packages/cli/binding/src/cli.rs | Added custom help printing with print_help() function, updated command descriptions, and added placeholder normalize_help_args() function |
| packages/global/snap-tests/cli-helper-message/snap.txt | Updated snapshot to reflect new help output format |
| packages/cli/snap-tests/command-helper/snap.txt | Updated snapshot to reflect new help output format for local CLI |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| fn normalize_help_args(args: Vec<String>) -> Vec<String> { | ||
| args |
There was a problem hiding this comment.
The normalize_help_args function is defined but currently does nothing (it just returns the args unchanged). This appears to be a placeholder that was created to mirror the structure in packages/global/binding/src/lib.rs where normalize_help_args performs actual normalization to convert "help" subcommands to "--help" flags. Either implement the normalization logic here for consistency or remove this function and call print_help directly from the appropriate location if this normalization is not needed for the local CLI.
| args | |
| args | |
| .into_iter() | |
| .map(|arg| if arg == "help" { "--help".to_string() } else { arg }) | |
| .collect() |
| @@ -92,7 +92,6 @@ pub enum CustomTaskSubcommand { | |||
| args: Vec<String>, | |||
| }, | |||
| /// Install command. | |||
There was a problem hiding this comment.
The description comment for the Install command was removed. While this change makes the documentation more concise, the removed comment provided useful context that the install command currently forwards to the package manager. Consider keeping a brief note about this forwarding behavior, as it helps developers understand the command's implementation.
| /// Install command. | |
| /// Install command (currently forwards to the package manager). |
| Commands::Info { package, field, json, pass_through_args } => { | ||
| let exit_status = PmCommand::new(cwd) | ||
| .execute(PmCommands::View { | ||
| package: package.clone(), | ||
| field: field.clone(), | ||
| json: *json, | ||
| pass_through_args: pass_through_args.clone(), | ||
| }) | ||
| .await?; | ||
| return Ok(exit_status); | ||
| } |
There was a problem hiding this comment.
The new Info command forwards to PmCommands::View but there's no test coverage demonstrating that the top-level vite info command works correctly. Consider adding a test case to verify that vite info <package> properly delegates to the package manager's view/info command, similar to how other package manager commands are tested in the snap-tests directory.
| args: Vec<String>, | ||
| }, | ||
| /// Package manager utilities | ||
| /// Forward command to the package manager. |
There was a problem hiding this comment.
The description "Forward command to the package manager." ends with a period, which is inconsistent with other command descriptions in the enum (e.g., line 448: "Execute a package binary without installing it as a dependency"). Remove the trailing period for consistency.
| /// Forward command to the package manager. | |
| /// Forward command to the package manager |
| {bold}lib{reset} Build library | ||
| {bold}migrate{reset} Migrate an existing project to Vite+ | ||
| {bold}cache{reset} Manage the task cache | ||
| {bold}new{reset} Generate a new project |
There was a problem hiding this comment.
The help text displays "new" as a command, but the actual command is defined as "Gen" in the Commands enum (line 467). While the JavaScript code intercepts both "new" and "gen" commands before reaching Rust, it would be more consistent to add an alias attribute to the Gen command definition to make it clear in the code that both names are supported. This would improve code clarity and make it easier for future maintainers to understand that "new" is an officially supported alias.
| {bold}new{reset} Generate a new project | |
| {bold}gen{reset} (alias: new) Generate a new project |
This PR updates the output of the help command. It groups commands into "Vite+Commands" and "Package Manager Commands" to make the separation obvious. It also reorders the commands roughly based on importance, and mirrors the original Vite's order (dev, build, etc.).
I also added the info command to forward
vite infoto package managers. This is one of the commands I use all the time, so I think it's good to have at the top level, especially since it takes many args and would be confusing.The output of the local cli was aligned as well, and looks like this now: https://github.com/voidzero-dev/vite-plus/blob/help-update/packages/cli/snap-tests/command-helper/snap.txt#L6