-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
Benibenj/registerContextKeyHandler #213135
Conversation
Co-authored-by: Benjamin Pasero <[email protected]>
Btw there are unit test failures, and I wonder: can we add unit tests for this? |
Co-authored-by: Benjamin Pasero <[email protected]>
Looking into adding tests... |
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.
I still feel something is off and in my testing I find bugs, around the "Open Changes" action.
When I make changes to an editor, the action should appear, but it does not. And when it is there, I see it on other editors too.
Can you do a smoke test to see if this behaves correctly? I feel this is around the fact that we do not listen to active editor changes for registered context keys unless a group is added.
Added test that fails with this bug and fixed the bug. |
* cleanup editor group context keys * Update src/vs/workbench/browser/parts/editor/editorPart.ts Co-authored-by: Benjamin Pasero <[email protected]> * context key on parts * Update global context keys * remove scoped keys on group removal * cleanup * first draft contexkt key registration * Make it a provider * Use group instead of active editor * getGroupContextKeyValue * doc * Fix merge error * 💄 --------- Co-authored-by: Benjamin Pasero <[email protected]>
Allows to register a context key provider to set the context keys for each editor group context and the global one