-
Notifications
You must be signed in to change notification settings - Fork 369
feat: add --disable-commands flag to hide and disable slash commands in TUI #2913
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -188,6 +188,10 @@ type appModel struct { | |
|
|
||
| appName string | ||
| appVersion string | ||
|
|
||
| // disabledCommands holds slash commands to hide and disable. | ||
| // Normalized to start with "/". | ||
| disabledCommands map[string]bool | ||
| } | ||
|
|
||
| // Transcriber is the speech-to-text interface used by the TUI. It is an | ||
|
|
@@ -230,6 +234,32 @@ func WithVersion(v string) Option { | |
| } | ||
| } | ||
|
|
||
| // WithDisabledCommands hides and disables the given slash commands so they | ||
| // are stripped from the command palette, the slash-command parser, and | ||
| // completion. Each entry is normalized to start with "/" (so "cost" and | ||
| // "/cost" are equivalent) and lower-cased to match the registered slash | ||
| // command names (so "/Cost" and "/cost" are equivalent). | ||
| func WithDisabledCommands(slashCommands []string) Option { | ||
| return func(m *appModel) { | ||
| if len(slashCommands) == 0 { | ||
| return | ||
| } | ||
| if m.disabledCommands == nil { | ||
| m.disabledCommands = make(map[string]bool, len(slashCommands)) | ||
| } | ||
| for _, c := range slashCommands { | ||
| c = strings.ToLower(strings.TrimSpace(c)) | ||
| if c == "" { | ||
| continue | ||
| } | ||
| if !strings.HasPrefix(c, "/") { | ||
| c = "/" + c | ||
| } | ||
| m.disabledCommands[c] = true | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // WithCommandBuilder builds the command categories shown in the command | ||
| // palette from the given function. It overrides the default command category | ||
| // builder. To include the default commands, the given function should call | ||
|
|
@@ -392,7 +422,26 @@ func (m *appModel) reapplyKeyboardEnhancements() { | |
| } | ||
|
|
||
| func (m *appModel) commandCategories() []commands.Category { | ||
| return m.buildCommandCategories(context.Background(), m) | ||
| categories := m.buildCommandCategories(context.Background(), m) | ||
| if len(m.disabledCommands) == 0 { | ||
| return categories | ||
| } | ||
| filtered := make([]commands.Category, 0, len(categories)) | ||
| for _, cat := range categories { | ||
| items := make([]commands.Item, 0, len(cat.Commands)) | ||
| for _, item := range cat.Commands { | ||
| if m.disabledCommands[item.SlashCommand] { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MEDIUM] Case-insensitive filtering silently fails for dynamic commands with uppercase names
For built-in commands this is harmless (they're hardcoded as lowercase strings like // pkg/tui/commands/commands.go (existing, not in diff)
SlashCommand: "/" + name, // agent command — name not lowercased
SlashCommand: "/" + skillName, // skill command — skillName not lowercasedIf any such name contains uppercase letters (e.g. Fix: lowercase if m.disabledCommands[strings.ToLower(item.SlashCommand)] { |
||
| continue | ||
| } | ||
| items = append(items, item) | ||
| } | ||
| if len(items) == 0 { | ||
| continue | ||
| } | ||
| cat.Commands = items | ||
| filtered = append(filtered, cat) | ||
| } | ||
| return filtered | ||
| } | ||
|
|
||
| // chatPageOpts returns the chat.PageOption slice derived from the current | ||
|
|
||
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.
[LOW]
WithDisabledCommandsis case-sensitive —/Costsilently fails to disable/costThe function normalizes leading slashes (e.g.
"cost"→"/cost") and trims whitespace, but does not fold to lowercase. If a programmatic caller passes"/Cost"or"/COST", the map lookupm.disabledCommands[item.SlashCommand]will not match the actual slash command (which is stored in lowercase). The command will silently remain enabled.The doc comment currently only mentions the slash-prefix normalization. In practice, all defined slash commands use lowercase, so interactive users of the CLI flag are unlikely to encounter this. But the
WithDisabledCommandspublic API gives no indication that casing matters.Suggestion: either add a note to the doc comment (
// Case-sensitive: entries must match the exact casing of the slash command.) or normalize withstrings.ToLower.