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

Debt: consolidate and cleanup context methods in editorCommands.ts #213155

Closed
bpasero opened this issue May 21, 2024 · 2 comments · Fixed by #219971
Closed

Debt: consolidate and cleanup context methods in editorCommands.ts #213155

bpasero opened this issue May 21, 2024 · 2 comments · Fixed by #219971
Assignees
Labels
debt Code quality issues unreleased Patch has not yet been released in VS Code Insiders workbench-editors Managing of editor widgets in workbench window
Milestone

Comments

@bpasero
Copy link
Member

bpasero commented May 21, 2024

We now have collected: getEditorsContext , resolveEditorsContext, getCommandsContext, resolveCommandsContext, getMultiSelectedEditorContexts in editorCommands.ts with each having some complexity.

Challenge for new actions is to know which one to use and for existing actions to call the right thing. Also there is questionable overlap with group identifiers and groups sitting next to each other, so we should consolidate.

@bpasero bpasero added the debt Code quality issues label May 21, 2024
@bpasero bpasero added this to the June 2024 milestone May 21, 2024
@bpasero bpasero added the workbench-editors Managing of editor widgets in workbench window label May 21, 2024
@benibenj
Copy link
Contributor

I'm seeing other areas using IEditorCommandsContext which possibly do not extract the context correctly so we need to check all these cases. Also, it's possible that the wrong multiselect context is used (list vs tabs) for example when focus is on open editors view but action is run on tabs. Maybe IEditorCommandsContext should include the source of the action so we do not have to figure it out by checking focus?

@bpasero
Copy link
Member Author

bpasero commented May 29, 2024

I would think a good outcome is if we had 1 method for commands to use and the others variants staying private. That ensures we have 1 way of unwrapping context for all consumers. Maybe certain behaviour should be configurable via options if we need specific behaviour, but then it becomes very explicit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debt Code quality issues unreleased Patch has not yet been released in VS Code Insiders workbench-editors Managing of editor widgets in workbench window
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants