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.

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

/help inline chat link doesn't work #598

Closed
gregvanl opened this issue Nov 21, 2023 · 15 comments · Fixed by microsoft/vscode#199831
Closed

/help inline chat link doesn't work #598

gregvanl opened this issue Nov 21, 2023 · 15 comments · Fixed by microsoft/vscode#199831
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug verified Verification succeeded

Comments

@gregvanl
Copy link

gregvanl commented Nov 21, 2023

  • VS Code Version: 1.85 Insiders, Copilot Chat pre-release
  • OS Version: Windows 11
  1. Bring up Chat view
  2. Run /help
  3. try to click the "starting an inline chat session" link

Inline chat isn't started in the open editor. Command ID inlineChat.start looks correct

image

Works with VS Code 1.84.2 and Copilot Chat stable

@github-actions github-actions bot added the triage-needed Issues needing to be assigned to the prospective feature owner label Nov 21, 2023
@lramos15 lramos15 assigned jrieken and alexdima and unassigned rebornix Nov 22, 2023
@lramos15 lramos15 removed the triage-needed Issues needing to be assigned to the prospective feature owner label Nov 22, 2023
@jrieken jrieken assigned roblourens and unassigned alexdima Nov 24, 2023
@jrieken
Copy link
Member

jrieken commented Nov 24, 2023

@roblourens The inlineChat.start command cannot be used like this, it requires editor focus (and an editor...) I'd suggestion to add a special command that first focuses an editor (or creates an untitled one) and then triggers the command. I believe this should be done inside the extension

@roblourens
Copy link
Member

@joyceerhl It doesn't seem helpful to open an untitled editor with the inline chat widget, should we have this just list the keyboard shortcut or tell users to click the sparkle button?

@joyceerhl
Copy link

This message only appears when the user has an active editor at the time that the chat welcome message is rendered, but of course it doesn't rerender when the active editor changes. I think we could go with listing the keyboard shortcut

@joyceerhl joyceerhl added the bug Issue identified by VS Code Team member as probable bug label Nov 27, 2023
@joyceerhl joyceerhl added this to the November 2023 milestone Nov 27, 2023
@joyceerhl
Copy link

Oh, actually, this text is still owned by Copilot Chat, and I don't think we should hardcode the keybinding. We don't yet allow extensions to look up the keybinding for a command, though there have been several issues filed about this microsoft/vscode#162433, microsoft/vscode#196835. I actually have a RemoteHub use case for being able to lookup keybindings microsoft/vscode-remote-repositories-github#366, I'll start an API proposal for that and remove the command link from the /help text in the meantime

@justschen justschen added the verified Verification succeeded label Nov 30, 2023
@joyceerhl joyceerhl reopened this Nov 30, 2023
@joyceerhl joyceerhl removed the verified Verification succeeded label Nov 30, 2023
@joyceerhl
Copy link

joyceerhl commented Nov 30, 2023

All the sparkle buttons are disabled by default for upcoming stable so the help text can't refer to them

@jrieken
Copy link
Member

jrieken commented Dec 1, 2023

We don't yet allow extensions to look up the keybinding for a command,

Shouldn't the markdown renderer simple insert keybindings? The MD refer to a command and all it takes is a lookup for the keybinding in the renderer. IMO no need for special API

@joyceerhl
Copy link

Do you mean to e.g. support the kb() syntax similar to how we add keybindings on the vscode-docs site? I would also like this tho I didn't think this was generally supported in markdown renderers. @mjbvz is supporting keybinding syntax in our markdown renderer something you'd be open to?

@joyceerhl
Copy link

joyceerhl commented Dec 1, 2023

Apparently GitHub uses <kbd></kbd> tags e.g. Ctrl+Enter (tho that won't help extensions since they will still have to hardcode)

@joyceerhl
Copy link

Filed upstream microsoft/vscode#199802

@roblourens
Copy link
Member

I think he's saying that vscode/ChatListRenderer should be responsible for seeing that we rendered a command link, looking up the keybinding, and doing something to render it, like adding it to the tooltip. Or it could do something smart like append it into the link text like (cmd+i).

kbd rendering is supported by our markdown renderer, the markdown preview uses it, but all the fancy stuff is disabled in chat.

@joyceerhl
Copy link

Hm, I don't know if injecting that everywhere is going to lead to weird/unexpected behavior. I was thinking of supporting kbstyle(inlineChat.start) directly in the markdown renderer instead, since it's unambiguous that you'd want the label to be rendered there, and we'd also be able to use this in more places than just chat

@roblourens
Copy link
Member

That would be cool, but in chat in particular I want to keep command links very very limited so it wouldn't cause an issue if the renderer just did it automatically.

@joyceerhl
Copy link

I got pretty far with supporting kbstyle and ran into the problem that the styles for our keybinding label renderer get stripped during markdown sanitization, and I don't think there's a way to allowlist specific styles using dompurify, so went with a scoped fix for chat as suggested

@jrieken
Copy link
Member

jrieken commented Dec 4, 2023

looking up the keybinding, and doing something to render it, like adding it to the tooltip

Yeah, I was thinking about the tooltip and not much more.

@joyceerhl What are you planning, not sure I can follow. Also, lets keep in mind that inlineChat.start is only useful when having an editor to begin with. Otherwise it's a noop (maybe command link rendering should also take enablement into account)

@joyceerhl
Copy link

We'll inject the keybinding in parentheses after the command link text now if a keybinding exists (in insiders after we release stable). The welcome message doesn't include the inline chat command link if there is no active editor, but the welcome message is only computed once and then restored from serialized state, so most of the time it is stale. Maybe we should update it when the active editor changes and/or always call the api for the latest data?

@roblourens roblourens modified the milestones: December 2023/January 2024, December / January 2024 Dec 14, 2023
@mjbvz mjbvz added the verified Verification succeeded label Jan 24, 2024
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Aug 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants