-
Notifications
You must be signed in to change notification settings - Fork 742
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
A few changes on the slash command #4268
Conversation
bmarty
commented
Oct 19, 2021
- Fixes Slash commands are not robust anymore to exceptions #4261 in 364654b
- Fixes /part doesn't part without parameters #2909 : Implement /part command> I've left /leave command there, even if it is not present in EleWeb. Leaving an unknown room will have no effect, I think we can improve that later, this is a very technical command.
- Add a loader when a slash command is running, and clear draft only in case of success (better UX)
- Also update some wording, translation can be updated later (no need to create a new string)
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.
Just one remark
} | ||
} else { | ||
ParsedCommand.ErrorSyntax(Command.PART) | ||
when (messageParts.size) { |
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.
It's not equivalent to the code you replace, is it ok?
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.
reason is removed here (iso EleWeb), if this is your concern.
popDraft() | ||
} | ||
is ParsedCommand.ChangeTopic -> { | ||
handleChangeTopicSlashCommand(slashCommandResult) | ||
popDraft() |
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.
is popDraft
not needed for these commands or was it the cause of the crash?
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.
popDraft is now handled by handleChangeTopicSlashCommand (in launchSlashCommandFlowSuspendable)
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 crash fix is just 364654b