Replace old actions with new actions that have configurable key bindings #4914

andrewyuq · 2024-09-25T19:37:12Z

Accept (Tab)
Force Accept (Opt + Tab/Opt + Enter)
Navigate to Previous (Opt + [)
Navigate to Next (Opt + ])

Notes:

sessionContext (a new data class in CodeWhispererModel.kt) and isDisplaySessionActive() (checks if we are showing codewhisperer) will be introduced in the following PRs.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)

Description

Checklist

My code follows the code style of this project
I have added tests to cover my changes
A short description of the change has been added to the CHANGELOG if the change is customer-facing in the IDE.
I have added metrics for my changes (if required)

License

I confirm that my contribution is made under the terms of the Apache 2.0 license.

rli · 2024-09-25T22:35:36Z

plugins/amazonq/codewhisperer/jetbrains-community/resources/META-INF/plugin-codewhisperer.xml

+        <action id="codewhisperer.inline.accept"
+                class="software.aws.toolkits.jetbrains.services.codewhisperer.actions.CodeWhispererAcceptAction"
+                text="Accept the Current Inline Suggestion" description="Accept the current inline suggestions">
+            <keyboard-shortcut keymap="$default" first-keystroke="TAB"/>


i'm a bit worried about the interaction between this and the IDE

The logic will be what the promoter does -- when the CW is showing:
CodeWhispererInvocationStatus.getInstance().isDisplaySessionActive()
it will be promoted to CW accept.
Otherwise it will be IDE tab action.

.../software/aws/toolkits/jetbrains/services/codewhisperer/actions/CodeWhispererAcceptAction.kt

rli · 2024-09-25T22:36:37Z

.../software/aws/toolkits/jetbrains/services/codewhisperer/actions/CodeWhispererAcceptAction.kt

+import software.aws.toolkits.jetbrains.services.codewhisperer.service.CodeWhispererInvocationStatus
+import software.aws.toolkits.resources.message
+
+open class CodeWhispererAcceptAction(title: String = message("codewhisperer.inline.accept")) : AnAction(title), DumbAware {


make sealed and move the force action here since there is no difference except the key binding?

It seems like I can't make CodeWhispererAcceptAction as sealed class because I will be getting this error:

Caused by: java.lang.NullPointerException: null cannot be cast to non-null type software.aws.toolkits.jetbrains.services.codewhisperer.actions.CodeWhispererAcceptAction

rli

failing lint
needs will or someone else to ack
probably should target a feature branch instead of main

rli · 2024-10-03T00:34:48Z

...software/aws/toolkits/jetbrains/services/codewhisperer/popup/CodeWhispererPopupComponents.kt

+            run {
+                // TODO: Doesn't reflect dynamically if users change but didn't restart IDE
+                val shortcut = ActionManager.getInstance().getAction("codewhisperer.inline.navigate.next")
+                    .shortcutSet.shortcuts.first()


will throw if user somehow removed shortcut

why not

KeymapUtil.getFirstKeyboardShortcutText

or getPrimaryShortcut

rli · 2024-10-03T00:38:19Z

...software/aws/toolkits/jetbrains/services/codewhisperer/popup/CodeWhispererPopupComponents.kt

+                if (SystemInfo.isMac) {
+                    MacKeymapUtil.getKeyStrokeText(keyStroke, " ", true)
+                } else {
+                    KeymapUtil.getKeystrokeText(keyStroke)
+                }


do we actually need a space delim?

I think I tried without space delim it looks a bit weird

rli · 2024-10-03T00:43:12Z

...software/aws/toolkits/jetbrains/services/codewhisperer/settings/CodeWhispererConfigurable.kt

+                    // workaround for certain cases for sometimes the string is not input there
+                    EdtExecutorService.getScheduledExecutorInstance().schedule({
+                        settings.select(configurable, Q_INLINE_KEYBINDING_SEARCH_TEXT)
+                    }, 500, TimeUnit.MILLISECONDS)


feels like this should not be required

last time I checked if I don't have this, it won't input the text in the keymap setting bar, I will try again later

rli · 2024-10-03T00:43:33Z

...software/aws/toolkits/jetbrains/services/codewhisperer/settings/CodeWhispererConfigurable.kt

+                    val settings = DataManager.getInstance().getDataContext(e.source as ActionLink).getData(Settings.KEY) ?: return@link
+                    val configurable: Configurable = settings.find("preferences.keymap") ?: return@link


user needs feedback if these are null

will add a TODO for now, can't think of a best way to inform users failures in this case

rli · 2024-10-03T00:43:45Z

...software/aws/toolkits/jetbrains/services/codewhisperer/settings/CodeWhispererConfigurable.kt

                        )
                        enabled(invoke)
-                    }.comment(message("aws.settings.codewhisperer.project_context_index_thread.tooltip"))
+                    }.comment(message("aws.settings.codewhisperer.project_context_index_thread.tooltip"), maxLineLength = 47)


where does magic number come from

I will just remove it for now.

Will-ShaoHua · 2024-10-03T23:23:13Z

...are/aws/toolkits/jetbrains/services/codewhisperer/actions/CodeWhispererNavigateNextAction.kt

+    override fun update(e: AnActionEvent) {
+        e.presentation.isEnabled = e.project != null && e.getData(CommonDataKeys.EDITOR) != null &&
+            CodeWhispererInvocationStatus.getInstance().isDisplaySessionActive()
+    }


nit

Suggested change

override fun update(e: AnActionEvent) {

e.presentation.isEnabled = e.project != null && e.getData(CommonDataKeys.EDITOR) != null &&

CodeWhispererInvocationStatus.getInstance().isDisplaySessionActive()

}

override fun update(e: AnActionEvent) {

e.presentation.isEnabled = e.project != null &&

e.getData(CommonDataKeys.EDITOR) != null &&

CodeWhispererInvocationStatus.getInstance().isDisplaySessionActive()

}

Will-ShaoHua · 2024-10-03T23:24:28Z

...are/aws/toolkits/jetbrains/services/codewhisperer/actions/CodeWhispererNavigatePrevAction.kt

+    override fun update(e: AnActionEvent) {
+        e.presentation.isEnabled = e.project != null && e.getData(CommonDataKeys.EDITOR) != null &&
+            CodeWhispererInvocationStatus.getInstance().isDisplaySessionActive()
+    }


nit

e.project != null && e.getData(CommonDataKeys.EDITOR) != null && CodeWhispererInvocationStatus.getInstance().isDisplaySessionActive()

Will-ShaoHua · 2024-10-03T23:31:54Z

...are/aws/toolkits/jetbrains/services/codewhisperer/actions/CodeWhispererNavigateNextAction.kt

+        ApplicationManager.getApplication().messageBus.syncPublisher(
+            CodeWhispererPopupManager.CODEWHISPERER_USER_ACTION_PERFORMED
+        ).navigateNext(sessionContext)


not related to this PR tho, i feel we could make this messagebus a project level

prob doesn't matter since we aren't going to have multple display sessions across different projects

1. Accept (Tab) 2. Force Accept (Opt + Tab/Opt + Enter) 3. Navigate to Previous (Opt + [) 4. Navigate to Next (Opt + ])

andrewyuq requested a review from rli September 25, 2024 19:37

andrewyuq marked this pull request as ready for review September 25, 2024 19:37

andrewyuq requested review from a team as code owners September 25, 2024 19:37

rli reviewed Sep 25, 2024

View reviewed changes

rli approved these changes Oct 3, 2024

View reviewed changes

andrewyuq changed the base branch from main to auto-trigger-revamp October 3, 2024 20:44

Will-ShaoHua reviewed Oct 3, 2024

View reviewed changes

Will-ShaoHua approved these changes Oct 3, 2024

View reviewed changes

Will-ShaoHua reviewed Oct 3, 2024

View reviewed changes

andrewyuq added 4 commits October 4, 2024 09:45

Replace old actions with new actions that have configurable key bindings

357fa36

1. Accept (Tab) 2. Force Accept (Opt + Tab/Opt + Enter) 3. Navigate to Previous (Opt + [) 4. Navigate to Next (Opt + ])

put sessionConext as project userdata and remove states of the actions

42dce38

add popup keybinding hint and settings hyperlink

64304f7

Addressing comments

6043c8e

andrewyuq force-pushed the main branch from b09143e to 6043c8e Compare October 4, 2024 17:30

andrewyuq merged commit 34b3435 into aws:auto-trigger-revamp Oct 4, 2024
0 of 8 checks passed

		val settings = DataManager.getInstance().getDataContext(e.source as ActionLink).getData(Settings.KEY) ?: return@link
		val configurable: Configurable = settings.find("preferences.keymap") ?: return@link

Replace old actions with new actions that have configurable key bindings #4914

Replace old actions with new actions that have configurable key bindings #4914

Uh oh!

Conversation

andrewyuq commented Sep 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Types of changes

Description

Checklist

License

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewyuq Sep 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rli left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rli Oct 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

andrewyuq commented Sep 25, 2024 •

edited

Loading

andrewyuq Sep 26, 2024 •

edited

Loading

rli Oct 3, 2024 •

edited

Loading