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

Benibenj/registerContextKeyHandler #213135

Merged
merged 29 commits into from
May 23, 2024
Merged

Conversation

benibenj
Copy link
Contributor

@benibenj benibenj commented May 21, 2024

Allows to register a context key provider to set the context keys for each editor group context and the global one

@benibenj benibenj self-assigned this May 21, 2024
@benibenj benibenj requested a review from bpasero May 21, 2024 16:26
@benibenj benibenj marked this pull request as ready for review May 21, 2024 16:29
@benibenj benibenj enabled auto-merge (squash) May 21, 2024 16:29
@vscodenpa vscodenpa added this to the May 2024 milestone May 21, 2024
src/vs/workbench/browser/parts/editor/editorParts.ts Outdated Show resolved Hide resolved
src/vs/workbench/browser/parts/editor/editorParts.ts Outdated Show resolved Hide resolved
src/vs/workbench/browser/parts/editor/editorParts.ts Outdated Show resolved Hide resolved
src/vs/workbench/browser/parts/editor/editorParts.ts Outdated Show resolved Hide resolved
src/vs/workbench/browser/parts/editor/editorParts.ts Outdated Show resolved Hide resolved
src/vs/workbench/browser/parts/editor/editorParts.ts Outdated Show resolved Hide resolved
@benibenj benibenj requested a review from bpasero May 22, 2024 14:52
@bpasero
Copy link
Member

bpasero commented May 22, 2024

benibenj and others added 2 commits May 23, 2024 09:12
Co-authored-by: Benjamin Pasero <benjamin.pasero@microsoft.com>
@benibenj
Copy link
Contributor Author

Looking into adding tests...

@benibenj benibenj requested a review from bpasero May 23, 2024 09:51
Copy link
Member

@bpasero bpasero left a 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.

@benibenj
Copy link
Contributor Author

Added test that fails with this bug and fixed the bug.

@benibenj benibenj merged commit ca45e3d into main May 23, 2024
6 checks passed
@benibenj benibenj deleted the benibenj/registerContextKeyHandler branch May 23, 2024 15:52
andremmsilva pushed a commit to PIC1G55/vscodeG55 that referenced this pull request May 26, 2024
* cleanup editor group context keys

* Update src/vs/workbench/browser/parts/editor/editorPart.ts

Co-authored-by: Benjamin Pasero <benjamin.pasero@microsoft.com>

* 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 <benjamin.pasero@microsoft.com>
@microsoft microsoft locked and limited conversation to collaborators Jul 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants