feat(MessageComposer): control command sendability#1746
Conversation
Add configurable command sendability validators so the composer can block submit for incomplete built-in and custom commands. Reuse the same validation context for send-button gating and final composition validation to keep command readiness consistent. Co-authored-by: Cursor <cursoragent@cursor.com>
Provide a default ban validator in composer config. Allow command validator arrays to be replaced via config overrides. Co-authored-by: Cursor <cursoragent@cursor.com>
MartinCupela
requested review from
arnautov-anton,
isekovanic,
oliverlaz,
santhoshvai,
szuperaz and
vishalnarkhede
as code owners
|
Size Change: +5.46 kB (+1.42%) Total Size: 390 kB 📦 View Changed
|
| return { command, ready: true }; | ||
| } | ||
|
|
||
| const reason = stripMentionTokens(commandArgsText, mentionedUsersInText); |
There was a problem hiding this comment.
I might be missing something here, but how would this generate a good reason for why the command is currently not sendable ?
| return { command, ready: true }; | ||
| }; | ||
| export const DEFAULT_COMMANDS_CONFIG: CommandsConfig = { | ||
| sendValidators: [defaultCommandSendabilityValidator], |
There was a problem hiding this comment.
Why do we keep these as an array ? Since the validation step is a middleware anyway, integrators can either add their own additional validation steps (or replace our current one)
| const getDisabledRawCommand = ( | ||
| composer: MessageComposer, | ||
| searchSource: CommandSearchSource, | ||
| searchSource: Pick<CommandSearchSource, 'query'>, |
There was a problem hiding this comment.
Why not just use CommandSearchSource here ? It should still comply
| const currentCommand = | ||
| composer.textComposer.command ?? | ||
| getCommandByName(effectiveCommandSearchSource, getRawCommandName(inputText)); | ||
| if (currentCommand) { |
There was a problem hiding this comment.
| if (currentCommand) { | |
| if (currentCommand && notifyCommandNotReady({ | |
| composer, | |
| sendability: composer.validateCommandSendability(currentCommand, inputText), | |
| })) { |
| this.pollId || | ||
| !!this.locationComposer.validLocation | ||
| const currentCommand = this.textComposer.command; | ||
| const commandIsSendable = !currentCommand || this.isCommandSendable(currentCommand); |
There was a problem hiding this comment.
Would it be possible to extract this in a separate getter please ?
So something like:
get isCommandSendable() {
const currentCommand = this.textComposer.command;
return !currentCommand || this.isCommandSendable(currentCommand);
}
| export const DEFAULT_COMMANDS_CONFIG: CommandsConfig = { | ||
| sendValidators: [defaultCommandSendabilityValidator], | ||
| }; | ||
| export const applyCommandValidatorOverride = ( |
There was a problem hiding this comment.
Since the command validation anyway goes through a middleware, this might not be needed I think. While convenient for integrators, I think convergence towards a middleware approach would be more appropriate (it would separate the need for overrides as well), unless I'm missing something important here
2 participants
Goal
Add API that allows to verify, whether the message composer in command mode has state ready to be composed and sent to the server.
This are the rules:
MessageComposer.isCommandSendablereturnstrueMessageComposer.isCommandSendablereturnstrueMessageComposer.isCommandSendablereturnstrueMessageComposer.isCommandSendablereturnstrueIt is possible to provide custom command sendability validation function(s) in case the integrators have custom commands and want to apply custom validation rules.
Mentioned users state kept up-to-date on text change
Make sure that the mentioned users array in text composer state is not stale. Until now, this was verified only at the composition time. Now we prune stale user mentions on every text change middleware execution.