Skip to content
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.

Sign up for GitHub

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

Fixing and simplifying New Chat keybindings logic #6514

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
+58 −41
Open

Fixing and simplifying New Chat keybindings logic #6514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
62e077f
Fixing and simplifying New Chat keybindings logic
arafatkatze Jan 3, 2025
7fa5bbd
Fixing and simplifying New Chat keybindings logic
arafatkatze Jan 3, 2025
46d78a1
Fixing and simplifying New Chat keybindings logic
arafatkatze Jan 3, 2025
f0f8d21
Fixing and simplifying New Chat keybindings logic
arafatkatze Jan 3, 2025
19b3e7e
Fixing and simplifying New Chat keybindings logic
arafatkatze Jan 3, 2025
f09f67d
Fixing and simplifying New Chat keybindings logic
arafatkatze Jan 5, 2025
50e5a4e
Fixing and simplifying New Chat keybindings logic
arafatkatze Jan 5, 2025
11cea67
Fixing and simplifying New Chat keybindings logic
arafatkatze Jan 5, 2025
59b79ef
Fixing and simplifying New Chat keybindings logic
arafatkatze Jan 5, 2025
855693d
Fixing and simplifying New Chat keybindings logic
arafatkatze Jan 5, 2025
3bca7c8
Fixing and simplifying New Chat keybindings logic
arafatkatze Jan 5, 2025
e5d7d3f
Fixing and simplifying New Chat keybindings logic
arafatkatze Jan 5, 2025
4ab276d
Fixing and simplifying New Chat keybindings logic
arafatkatze Jan 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions vscode/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -610,11 +610,21 @@
{
"command": "cody.chat.toggle",
"key": "alt+l",
"when": "cody.activated && editorTextFocus",
"mac": "alt+l",
"when": "cody.activated && editorHasSelection",
"args": {
"editorFocus": true
}
},
{
"command": "cody.chat.toggle",
"key": "alt+l",
"mac": "alt+l",
"when": "cody.activated && !editorHasSelection",
"args": {
"editorFocus": false
}
},
{
"command": "cody.chat.toggle",
"key": "alt+l",
Expand All @@ -625,38 +635,28 @@
},
{
"command": "cody.chat.toggle",
"key": "alt+/",
"key": "alt+l",
"when": "cody.activated && editorTextFocus",
"args": {
"editorFocus": true
}
},
{
"command": "cody.chat.toggle",
"key": "alt+/",
"key": "alt+l",
"when": "cody.activated && !editorTextFocus",
"args": {
"editorFocus": false
}
},
{
"command": "cody.mention.selection",
"key": "shift+alt+l",
"when": "cody.activated && editorTextFocus && editorHasSelection"
},
{
"command": "cody.mention.selection",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should aim to keep some basic compatibility with older commands where possible. One option is to keep the command IDs, but they are just aliases for the main chat command. People may have configured custom shortcuts for these IDs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One option is to keep the command IDs, but they are just aliases for the main chat command. People may have configured custom shortcuts for these IDs.

Yeah I suppose we can keep them(so that's what I will do). Personally I think we probably shouldn't have introduced so much optionality in the first place. Too many options is too much confusion but I suppose we should just keep these things to maintain compatibility.

"key": "shift+alt+/",
"key": "alt+shift+l",
"when": "cody.activated && editorTextFocus && editorHasSelection"
},
{
"command": "cody.chat.new",
"key": "shift+alt+l",
"when": "cody.activated && !editorTextFocus"
},
{
"command": "cody.chat.new",
"key": "shift+alt+/",
"key": "alt+l",
"when": "cody.activated && !editorTextFocus"
},
{
Expand Down
43 changes: 19 additions & 24 deletions vscode/src/chat/chat-view/ChatsController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,18 @@ export class ChatsController implements vscode.Disposable {
)
}

public async clearEditorText(): Promise<void> {
const webviewPanelOrView = this.panel.webviewPanelOrView
if (!webviewPanelOrView) {
return
}

webviewPanelOrView.webview.postMessage({
type: 'clientAction',
clearEditorText: true,
})
}

public registerViewsAndCommands() {
this.disposables.push(
vscode.window.registerWebviewViewProvider('cody.chat', this.panel, {
Expand All @@ -141,7 +153,6 @@ export class ChatsController implements vscode.Disposable {
return undefined
}
}

this.disposables.push(
vscode.commands.registerCommand('cody.chat.moveToEditor', async () => {
localStorage.setLastUsedChatModality('editor')
Expand Down Expand Up @@ -189,35 +200,19 @@ export class ChatsController implements vscode.Disposable {
case 'editor':
return vscode.commands.executeCommand('cody.chat.newEditorPanel', args)
case 'sidebar':
return vscode.commands.executeCommand('cody.chat.newPanel', args)
return vscode.commands.executeCommand('cody.chat.toggle', args)
}
}),

vscode.commands.registerCommand(
'cody.chat.toggle',
async (ops: { editorFocus: boolean }) => {
const modality = getNewChatLocation()

if (ops.editorFocus) {
if (modality === 'sidebar') {
await vscode.commands.executeCommand('cody.chat.focus')
} else {
const editorView = this.activeEditor?.webviewPanelOrView
if (editorView) {
revealWebviewViewOrPanel(editorView)
} else {
vscode.commands.executeCommand('cody.chat.newEditorPanel')
}
}
async (args: { uri: URI; editorFocus: boolean }) => {
if (this.panel.isEmpty() && !args?.editorFocus) {
await this.clearEditorText()
} else {
if (modality === 'sidebar') {
await vscode.commands.executeCommand(
'workbench.action.focusActiveEditorGroup'
)
} else {
await vscode.commands.executeCommand('workbench.action.navigateEditorGroups')
}
vscode.commands.executeCommand('cody.chat.newPanel')
}
await vscode.commands.executeCommand('cody.chat.focus')
this.sendEditorContextToChat(args?.uri)
}
),
vscode.commands.registerCommand('cody.chat.history.export', () => this.exportHistory()),
Expand Down
1 change: 1 addition & 0 deletions vscode/src/chat/protocol.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ export type ExtensionMessage =
| { text: string; mode?: PromptMode | undefined | null; autoSubmit: boolean }
| undefined
| null
clearEditorText?: boolean | undefined | null
}
| ({ type: 'attribution' } & ExtensionAttributionMessage)
| { type: 'rpc/response'; message: ResponseMessage }
Expand Down
21 changes: 21 additions & 0 deletions vscode/webviews/chat/cells/messageCell/human/editor/HumanMessageEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ export const HumanMessageEditor: FunctionComponent<{
submitHumanInput,
setLastHumanInputIntent,
setPromptAsInput,
clearEditorText,
}) => {
const updates: Promise<unknown>[] = []

Expand Down Expand Up @@ -349,6 +350,26 @@ export const HumanMessageEditor: FunctionComponent<{

let promptIntent = undefined

if (clearEditorText) {
updates.push(
new Promise<void>(resolve => {
firstValueFrom(
extensionAPI.defaultContext().pipe(skipPendingOperation())
).then(({ initialContext }) => {
firstValueFrom(
extensionAPI.hydratePromptMessage('', initialContext)
).then(emptyState => {
if (editorRef.current) {
editorRef.current.setEditorState(emptyState)
editorRef.current.setFocus(true)
}
resolve()
})
})
})
)
}

if (setPromptAsInput) {
// set the intent
promptIntent = promptModeToIntent(setPromptAsInput.mode)
Expand Down
2 changes: 1 addition & 1 deletion vscode/webviews/chat/components/WelcomeFooter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const chatTips: ChatViewTip[] = [
vsCodeOnly: true,
},
{
message: 'Start a new chat with ⇧ ⌥ L or switch to chat with ⌥ /',
message: 'Press ⌥ L to start a new chat or switch to chat view',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keeping the option+L shortcut here but it's shift+option+L above? The "or switch to" reads as a different thing "command does X, or do Y" instead of "command X does A+B".

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I am aware of that. and I mentioned it in the video too but basically I think if I just do Press ⌥ L to start a new chat that would be succinct and do that job.

Copy link
Contributor

@ichim-david ichim-david Jan 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arafatkatze @olafurpg I'm just an user who has made a few contributions to this repo as such my input might not be worth much but I think Options + L should focus on the last input of the existing chat and hitting it again would toggle the panel visibility.

This is how it happens with Windsurf and also to some degree with Cursor (it toggles the panel visibility but starts a new conversation).
In Windsurf hitting shift + option + L starts a new conversation exactly like how Cody did it up to this point and hitting option + L simply focuses in the last input box and toggles the panel visibility.

I would also like to caution against changing the shortcut from option + L to shift + option + L as users might have changed it or have been used to having option + L keyboard shortcut.

The only problem I see in the current behavior is when you select a text and you get the ghost text that lets you know you can edit it with option + k or option + L to chat.
When I press option + L I would expect for what I had selected in the editor to be added to the chat input and currently what it does is to simply focus on the last input from the opened chat panel.
option-l-chat

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ichim-david thank you for your input! The shortcut will remain Option+L, and I left another comment about keeping compatibility with existing command IDs for people who have registered custom shortcuts.

On the exact behavior, I'm torn on the behavior of Option+L when you have a text selection, and I'm curious to hear what you think.

My feeling is that the "add to chat" behavior isn't helpful because chats already include the text selection by default. You end up with duplicate chips. I still think it's useful to display the shortcut in the decoration to make it easier to discover for users.

Copy link
Contributor

@ichim-david ichim-david Jan 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ichim-david thank you for your input! The shortcut will remain Option+L, and I left another comment about keeping compatibility with existing command IDs for people who have registered custom shortcuts.

On the exact behavior, I'm torn on the behavior of Option+L when you have a text selection, and I'm curious to hear what you think.

My feeling is that the "add to chat" behavior isn't helpful because chats already include the text selection by default. You end up with duplicate chips. I still think it's useful to display the shortcut in the decoration to make it easier to discover for users.

Since hitting option + k will only affect the selected text I would expect the same to happen if I hit the shortcut to have it added to the chat input.
Unfortunately, you get the current file chip added before the selection and even if you get rid of it as soon as you modify the selection the current file chip comes back.

Depending on where you are coming from (let us say you don't have the chat window open, hitting option + L and seeing both the full file and the selection added is an anti-pattern as I wouldn't expect that to happen).

See this short screencast for what I mean
https://github.com/user-attachments/assets/63ed9645-337e-4a8c-9dbb-64763ecd0461

A small nit is the fact that if you delete the context and switch between windows nothing happens but I don't expect many users to try this and complain.

Hopefully, this is helpful for you @olafurpg

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for sharing the video! The original motivation to kickstart this PR was that the current behavior is both buggy and confusing, and we want to simplify things so you only need to remember a single simple shortcut like option+L and it does what most people want most of the time. We can optionally expose commands for more narrow use-cases that have no defined shortcuts, which is helpful for power users.

I'm leaning towards that option+L with selected text should "freeze" that selection in the input. We currently have a dynamic chip that updates alongside the text selection. When you trigger the shortcut with text selection, then the dynamic chip is replaced with a hardcoded chip. However, I don't think it's urgent to implement this behavior in the first PR. To start with, we should just make Option+L reliably open the chat.

Copy link
Contributor

@ichim-david ichim-david Jan 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@olafurpg here is one last video where I show how Cody add to chat differs from Windsurf and Cursor, perhaps it might be useful as feedback for you guys for any kind of future considerations.

command-l-behavior.mp4

EDIT:
I forgot to show in the video that repeated use of command + L toggles the chat panel visibility
in the same way it happens with Windsurf.

icon: MessageSquarePlus,
vsCodeOnly: false,
},
Expand Down
2 changes: 1 addition & 1 deletion vscode/webviews/tabs/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@ export function getCreateNewChatCommand(options: NewChatCommandInput): string {
return IDE === CodyIDE.Web
? 'cody.chat.new'
: webviewType === 'sidebar' || !multipleWebviewsEnabled
? 'cody.chat.newPanel'
? 'cody.chat.toggle'
: 'cody.chat.newEditorPanel'
}
Loading