Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves recipe slash command parsing by treating all text after the command as a single parameter value instead of splitting on whitespace. It adds validation to prevent recipes with multiple required parameters from being used via slash commands, guiding users to the CLI's goose run --recipe with --params instead.
- Changes parameter parsing to pass the full parameter string (spaces included) to recipes
- Adds recipe validation to count required parameters and enforce single-parameter restriction
- Improves error messages to guide users when recipes have too many required parameters
| .map(|params| params.iter().filter(|p| p.default.is_none()).count()) | ||
| .unwrap_or(0); | ||
|
|
||
| if params_without_default <= 1 { |
There was a problem hiding this comment.
When there are no required parameters (params_without_default == 0) but the user provides input, the code creates a vec with params_str. This will cause the parameter to be passed to a recipe that doesn't expect any positional parameters. The condition should be params_without_default == 1 instead of <= 1 to avoid this issue.
| if params_without_default <= 1 { | |
| if params_without_default == 1 { |
| .map(|params| params.iter().filter(|p| p.default.is_none()).count()) | ||
| .unwrap_or(0); | ||
|
|
||
| if params_without_default <= 1 { |
There was a problem hiding this comment.
When a recipe has only optional parameters (all with defaults) and the user provides a value, this code will pass that value as the first parameter. This could cause unexpected behavior since the recipe doesn't require any parameters. Consider checking if params_without_default is 0 and params_str is not empty, and either ignore the value or return an informative error.
| if params_without_default <= 1 { | |
| if params_without_default == 0 { | |
| let error_message = format!( | |
| "The /{} recipe does not require any parameters, \ | |
| but you provided: `{}`.\n\n\ | |
| Slash command recipes only support passing at most one \ | |
| required parameter.\n\n\ | |
| **To customize this recipe's optional parameters,** \ | |
| run it directly as a recipe (for example from the recipes sidebar).", | |
| command, | |
| params_str | |
| ); | |
| return Err(anyhow!(error_message)); | |
| } else if params_without_default == 1 { |
2 participants
Summary
Currently we split parameters at spacebar, and only the first word is sent to the first param.
Now everything is parsed as the first param (without a default, or just the first if they all have defaults).
If there are more than 2 required params, throw an error; I think this case is pretty hard to handle without some richer parameter filled UI; Since you aren't directly looking at the recipe you have little hope of remembering all the param names.