-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Add a built-in /init slash command #7381
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! I've reviewed the changes and have some suggestions for improvement. Overall, this is a clean implementation of built-in commands with good UI handling and comprehensive i18n support.
| init: { | ||
| name: "init", | ||
| description: "Initialize a project with recommended rules and configuration", | ||
| content: `Please analyze this codebase and create an AGENTS.md file containing: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The content for the init command is quite lengthy (87 lines). Would it make sense to externalize this content to a separate file or break it down into smaller, more focused commands? This would improve maintainability.
| /** | ||
| * Get all built-in commands as Command objects | ||
| */ | ||
| export async function getBuiltInCommands(): Promise<Command[]> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding JSDoc comments to these exported functions to help future developers understand how to add new built-in commands and what each function does.
| /** | ||
| * Get a specific built-in command by name | ||
| */ | ||
| export async function getBuiltInCommand(name: string): Promise<Command | undefined> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While the current implementation is simple, consider adding error handling for future built-in commands that might need validation or async operations. Could we make these functions more robust?
| const emptyCommand = await getBuiltInCommand("") | ||
| expect(emptyCommand).toBeUndefined() | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test coverage is good for basic functionality. Consider adding tests for edge cases like:
- Concurrent access to built-in commands
- Command name conflicts with user-defined commands
- Behavior when BUILT_IN_COMMANDS is modified at runtime
| <Settings className="w-3 h-3" /> | ||
| {t("chat:slashCommands.builtInCommands")} | ||
| </div> | ||
| {builtInCommands.map((command) => ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice implementation! Consider adding a visual indicator (like a small badge or icon) next to built-in commands to make it clearer that these are system-provided commands, in addition to the Settings icon in the section header.
| results.push({ | ||
| type: ContextMenuOptionType.SectionHeader, | ||
| label: "Custom Commands", | ||
| label: "Commands", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good change from "Custom Commands" to "Commands" to be more inclusive of built-in commands. This makes the UI more consistent.
| If the update_todo_list tool is available, create a todo list with these focused analysis steps: | ||
| 1. Quick scan for existing docs | ||
| - AI assistant rules (.cursorrules, CLAUDE.md, AGENTS.md, .roorules) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a potential typographical error: the text mentions "AI assistant rules (.cursorrules, CLAUDE.md, AGENTS.md, .roorules)". Should '.roorules' be '.cursor/rules' (or a similar variant) for consistency?
| - AI assistant rules (.cursorrules, CLAUDE.md, AGENTS.md, .roorules) | |
| - AI assistant rules (.cursorrules, CLAUDE.md, AGENTS.md, .roo/rules) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
Important
Adds a built-in
/initslash command for codebase initialization and updates command handling to support built-in commands./initinbuilt-in-commands.tsfor initializing codebase analysis.getCommands()andgetCommand()incommands.tsto include built-in commands with lowest priority.SlashCommandItem.tsx.built-in-commands.spec.ts.frontmatter-commands.spec.ts.SlashCommandsList.tsxto display built-in commands under a new section.chat.jsonfiles.command-integration.spec.tsto recognizebuilt-inas a command source.argumentHintto command properties inwebviewMessageHandler.ts.This description was created by
for 75cac18. You can customize this summary. It will automatically update as commits are pushed.