-
Notifications
You must be signed in to change notification settings - Fork 36k
Add support for copying math source from katex blocks #271590
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
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.
Pull Request Overview
This PR adds support for copying LaTeX source code from KaTeX math elements in chat responses. Users can now right-click on rendered math formulas and select "Copy Math Source" to get the underlying LaTeX code.
Key changes:
- Adds a new context key to detect when KaTeX math elements are focused
- Implements context detection for KaTeX elements in the chat widget's context menu
- Creates a new action to extract and copy LaTeX source from KaTeX annotations
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/common/chatContextKeys.ts | Adds new context key isKatexMathElement to identify when KaTeX elements are focused |
| src/vs/workbench/contrib/chat/browser/chatWidget.ts | Implements KaTeX element detection in context menu handler and sets the context key |
| src/vs/workbench/contrib/chat/browser/actions/chatCopyActions.ts | Adds new "Copy Math Source" action that extracts LaTeX from KaTeX annotation elements |
src/vs/workbench/contrib/chat/browser/actions/chatCopyActions.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
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.
Seems reasonable to me. Thanks for looking into this!
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.
Looks a little fragile to me, but @mjbvz had a look
Happy to discuss any changes you think would improve the stability here! Probably would be nicer to not have to do the |
|
Dealing with classes like this is basically touching internals of katex that could change in the future and therefore brick this feature on our end without any indication. Unless they have committed to the DOM structure that is. |
That makes sense, to my knowledge the class name has been around basically since the beginning so it should be relatively stable (but not guaranteed). This is good feedback though, I can check if we can inject a custom attribute at the same element and use that instead in a follow-up PR. Thanks! Edit: I did some more research, and it doesn't seem trivial to inject custom attributes or class names without hacky methods like string replacements or parsing. Given that we version lock the KaTeX component, I think it would be safer to leave it as is and make sure that whenever we update KaTeX that we also validate these scenarios. That being said, since the class name has been around for a long time I suspect that we should be safe for quite a while. |
|
A custom attribute with the source would be more reliable. We do lock it, but we don't go and verify every little feature on upgrading. |
Screenshot
Fixes #258286