Fixing and simplifying New Chat keybindings logic #6514

arafatkatze · 2025-01-03T09:59:03Z

Test plan

Open the debugger and just test all things chat with alt+l command which is very simple or select something and try the shift+alt+l to add to chat

Changelog

olafurpg · 2025-01-03T12:04:40Z

vscode/package.json

@@ -608,57 +608,16 @@
        "when": "!cody.activated"
      },
      {
-        "command": "cody.chat.toggle",
+        "command": "cody.chat.simpleNewChat",


nit: the words "simple" and "new" are in my deny-list of names to use in code. The "new" thing always eventually becomes old, and the "simple" is almost never simple in the end.

olafurpg · 2025-01-03T12:05:34Z

vscode/package.json

      },
      {
        "command": "cody.mention.selection",
        "key": "shift+alt+l",
        "when": "cody.activated && editorTextFocus && editorHasSelection"
      },
-      {
-        "command": "cody.mention.selection",


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.

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.

olafurpg · 2025-01-03T12:06:19Z

vscode/src/commands/GhostHintDecorator.ts

@@ -15,7 +15,7 @@ import type { SyntaxNode } from 'web-tree-sitter'
 import { execQueryWrapper } from '../tree-sitter/query-sdk'

 const EDIT_SHORTCUT_LABEL = isMacOS() ? 'Opt+K' : 'Alt+K'
-const CHAT_SHORTCUT_LABEL = isMacOS() ? 'Opt+L' : 'Alt+L'
+const CHAT_SHORTCUT_LABEL = isMacOS() ? 'Shift+Opt+L' : 'Shift+Alt+L'


Is it intentional to require the shift modifier here? Our primary chat shortcut should ideally only be option+l or command+l

So basically I have two two different chat shortcuts here.

New chat which is Alt+L

Add to chat when you select some text and that is Shift+Alt+L

See https://www.loom.com/share/5e306ff626b145c891e79c1fa016d9e4 for explaination

olafurpg · 2025-01-03T12:07:34Z

vscode/src/chat/chat-view/ChatsController.ts

-                }
-            ),
+            vscode.commands.registerCommand('cody.chat.simpleNewChat', async () => {
+                await this.panel.clearAndRestartSession()


I have not tested this locally, but this seems to unconditionally clear the session, which is not the desired behavior. We should only restart the session when 1) the sidebar is focused and 2) when the text input is non-empty. We also need to handle the modality here, and I'm not fully sure what to do there.

We should only restart the session when 1) the sidebar is focused

So when the sidebar is NOT focused on you press keybindings for new chat you move to a chat but you don't Actually start the new chat? isn't that moot then?

when the text input is non-empty.

So this one will require some webview manipulation because instead of checking "text is non-empty" we check if history is non empty and we could do that but I am not entirely sure if that's a good idea. The new chat keybindings should be very simple. Whenever you press then you either move to the new chat view that's just an empty chat or you start a new chat.

We also need to handle the modality here

I would disagree because that's the essence of the confusion. When you have editor chat do X and when you have sidebar chat do Y is just bad confusing behavior which was what you pointed out in the original slack thread.

Can't we get rid of it all and just do new chat= sidebar's new chat

The proposal here had clear instructions on the desired behavior, and it includes testing things like whether the chat input is empty or not.

Can't we get rid of it all and just do new chat= sidebar's new chat

I am OK with this, and it's roughly what I alluded to on Slack. Chat editors are a great feature but for the Option+L shortcut I think it simplifies a lot if it only works with the sidebar. When you expand a chat into the editor then this chat goes out of the shortcut "system"

olafurpg · 2025-01-03T12:08:10Z

vscode/src/commands/GhostHintDecorator.ts

@@ -130,7 +130,7 @@ const HINT_DECORATIONS: Record<
    { text: string; decoration: vscode.TextEditorDecorationType }
 > = {
    EditOrChat: {
-        text: `${EDIT_SHORTCUT_LABEL} to Edit, ${CHAT_SHORTCUT_LABEL} to Chat`,
+        text: `${EDIT_SHORTCUT_LABEL} to Edit, ${CHAT_SHORTCUT_LABEL} to Add to Chat`,


"add to" indicates this is a different thing. We should just have a single chat command.

This for the ghost text which shows when you select something so you want to "add to the existing chat" by adding the user selection as the context so that's what this does.

"add to" indicates this is a different thing

So this is a different thing.

We should just have a single chat command.

So then I can just remove it and go ${CHAT_SHORTCUT_LABEL} to start new Chat and then stop doing this "add to chat" thing but I think its good to have a new chat and then another "add to chat" option. Those are decent choices and its clean.

Try locally and you will see.

olafurpg · 2025-01-03T12:09:34Z

vscode/webviews/chat/components/WelcomeFooter.tsx

@@ -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',


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".

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.

@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.

@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.

@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

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.

@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.

arafatkatze · 2025-01-05T13:54:29Z

olafurpg · 2025-01-06T10:37:19Z

Thanks @arafatkatze !

Discard non-empty input and start new chat if sidebar is open

I'm unable to reproduce this behavior when I run it locally, the alt-L shortcut has no effect when have a non-empty input. The "New Chat" button also does not work anymore, it has no effect.

arafatkatze · 2025-01-06T12:44:55Z

Thanks for the feedback @olafurpg! I've fixed the "New Chat" button issue.

Discard non-empty input and start new chat if sidebar is open. I'm unable to reproduce this behavior when I run it locally, the alt-L shortcut has no effect when have a non-empty input

Regarding the alt-L behavior, I have a quick UX question to ensure we implement the right pattern:
When a user presses alt-L with selected text in the editor, which behavior would you prefer?

Always start a fresh new chat
Focus the existing chat and add the selection as context (treating alt-L as "I want to chat about this"). This is current behavior and will NOT clean the unsent chat.

I can implement either approach - just want to make sure we choose the most intuitive one for users.

arafatkatze added 5 commits January 3, 2025 13:58

Fixing and simplifying New Chat keybindings logic

62e077f

Fixing and simplifying New Chat keybindings logic

7fa5bbd

Fixing and simplifying New Chat keybindings logic

46d78a1

Fixing and simplifying New Chat keybindings logic

f0f8d21

Fixing and simplifying New Chat keybindings logic

19b3e7e

olafurpg requested changes Jan 3, 2025

View reviewed changes

arafatkatze added 7 commits January 5, 2025 13:23

Fixing and simplifying New Chat keybindings logic

f09f67d

Fixing and simplifying New Chat keybindings logic

50e5a4e

Fixing and simplifying New Chat keybindings logic

11cea67

Fixing and simplifying New Chat keybindings logic

59b79ef

Fixing and simplifying New Chat keybindings logic

855693d

Fixing and simplifying New Chat keybindings logic

3bca7c8

Fixing and simplifying New Chat keybindings logic

e5d7d3f

arafatkatze requested a review from olafurpg January 5, 2025 13:58

Fixing and simplifying New Chat keybindings logic

4ab276d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing and simplifying New Chat keybindings logic #6514

Fixing and simplifying New Chat keybindings logic #6514

arafatkatze commented Jan 3, 2025 •

edited

Loading

olafurpg Jan 3, 2025

olafurpg Jan 3, 2025

arafatkatze Jan 5, 2025

olafurpg Jan 3, 2025

arafatkatze Jan 3, 2025

olafurpg Jan 3, 2025

arafatkatze Jan 3, 2025

olafurpg Jan 3, 2025 •

edited

Loading

olafurpg Jan 3, 2025

olafurpg Jan 3, 2025

arafatkatze Jan 3, 2025 •

edited

Loading

arafatkatze Jan 3, 2025 •

edited

Loading

olafurpg Jan 3, 2025

arafatkatze Jan 3, 2025

ichim-david Jan 3, 2025 •

edited

Loading

olafurpg Jan 3, 2025

ichim-david Jan 3, 2025 •

edited

Loading

olafurpg Jan 3, 2025

ichim-david Jan 3, 2025 •

edited

Loading

arafatkatze commented Jan 5, 2025

olafurpg commented Jan 6, 2025

arafatkatze commented Jan 6, 2025 •

edited

Loading

Fixing and simplifying New Chat keybindings logic #6514

Are you sure you want to change the base?

Fixing and simplifying New Chat keybindings logic #6514

Conversation

arafatkatze commented Jan 3, 2025 • edited Loading

Test plan

Changelog

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olafurpg Jan 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arafatkatze Jan 3, 2025 • edited Loading

Choose a reason for hiding this comment

arafatkatze Jan 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ichim-david Jan 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ichim-david Jan 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ichim-david Jan 3, 2025 • edited Loading

Choose a reason for hiding this comment

arafatkatze commented Jan 5, 2025

Primary Shortcut Requirements

Core Functionality

Sidebar and Editor Management

Command Palette Integration

Outstanding Issues (To Be Addressed Separately)

olafurpg commented Jan 6, 2025

arafatkatze commented Jan 6, 2025 • edited Loading

arafatkatze commented Jan 3, 2025 •

edited

Loading

olafurpg Jan 3, 2025 •

edited

Loading

arafatkatze Jan 3, 2025 •

edited

Loading

arafatkatze Jan 3, 2025 •

edited

Loading

ichim-david Jan 3, 2025 •

edited

Loading

ichim-david Jan 3, 2025 •

edited

Loading

ichim-david Jan 3, 2025 •

edited

Loading

arafatkatze commented Jan 6, 2025 •

edited

Loading