-
Notifications
You must be signed in to change notification settings - Fork 251
fix(amazonq): Support chat quick action commands #5700
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
...rains-community/src/software/aws/toolkits/jetbrains/services/cwc/commands/ActionRegistrar.kt
Fixed
Show fixed
Hide fixed
.../software/aws/toolkits/jetbrains/services/amazonq/lsp/model/aws/chat/GenericCommandParams.kt
Dismissed
Show dismissed
Hide dismissed
.../software/aws/toolkits/jetbrains/services/amazonq/lsp/model/aws/chat/GenericCommandParams.kt
Fixed
Show fixed
Hide fixed
...rains-community/src/software/aws/toolkits/jetbrains/services/cwc/commands/ActionRegistrar.kt
Outdated
Show resolved
Hide resolved
.../software/aws/toolkits/jetbrains/services/amazonq/lsp/model/aws/chat/GenericCommandParams.kt
Show resolved
Hide resolved
.../software/aws/toolkits/jetbrains/services/amazonq/lsp/model/aws/chat/GenericCommandParams.kt
Show resolved
Hide resolved
.../software/aws/toolkits/jetbrains/services/amazonq/lsp/model/aws/chat/GenericCommandParams.kt
Show resolved
Hide resolved
...rains-community/src/software/aws/toolkits/jetbrains/services/cwc/commands/ActionRegistrar.kt
Fixed
Show fixed
Hide fixed
.../software/aws/toolkits/jetbrains/services/amazonq/lsp/model/aws/chat/GenericCommandParams.kt
Dismissed
Show dismissed
Hide dismissed
...rains-community/src/software/aws/toolkits/jetbrains/services/cwc/commands/ActionRegistrar.kt
Outdated
Show resolved
Hide resolved
_messages.tryEmit(ContextMenuActionMessage(command, project)) | ||
} else { | ||
// new agentic chat route | ||
runBlocking { |
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.
this will block edt
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.
well if I mark reportMessageClick
as suspend it would require me to mark other functions as suspend
.
Should I launch with coroutine?
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.
i really don't like ActionRegistrar
let's just do
ApplicationManager.getApplication().executeOnPooledThread {
runBlocking {
...
}
}
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.
maybe try this https://youtrack.jetbrains.com/issue/IJPL-371/Provide-currentThreadCoroutineScope-inside-AnActionactionPerformed
you can request a coroutine scope from the platform in the action system
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.
maybe like
currentThreadCoroutineScope().launch(getCoroutineBgContext()) {
ActionRegistrar.instance.reportMessageClick
}
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.
We can skip ActionRegistrar for these and handle it in custom action
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.
skip ActionRegistrar for these and handle it in custom action
This would require more lines of code.
…kits/jetbrains/services/cwc/commands/ActionRegistrar.kt Co-authored-by: Richard Li <742829+rli@users.noreply.github.com>
Types of changes
Description
ref: https://github.com/aws/aws-toolkit-vscode/pull/7169/files
Checklist
License
I confirm that my contribution is made under the terms of the Apache 2.0 license.