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.

Sign up for GitHub

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

Jump to bottom

Add open context to certain commands #110475

Merged
merged 5 commits into from
Nov 12, 2020
Merged

Add open context to certain commands #110475

merged 5 commits into from
Nov 12, 2020

Conversation

bpasero
Copy link
Member

@bpasero bpasero commented Nov 12, 2020 •
edited
Loading

This PR does 2 things:

  • moves the commands _workbench.open and friends into core workbench (this helps to depend on the command IDs from core parts in the future)
  • allows to pass IOpenEvent as argument to these commands in case invoked from trees

This PR fixes #85636

PS: I also made EditorViewColumn a core thing as well (renamed to EditorGroupColumn)

@bpasero bpasero added this to the November 2020 milestone Nov 12, 2020
@bpasero bpasero requested review from jrieken and alexr00 November 12, 2020 13:18
@bpasero bpasero self-assigned this Nov 12, 2020
@bpasero bpasero mentioned this pull request Nov 12, 2020
Closed
@bpasero
Copy link
Member Author

bpasero commented Nov 12, 2020

https://github.com/microsoft/vscode/pull/110475/files#diff-ee44b7286aa52f5c5a2737955cebe3a73945354241d92bcfd8708cca313600b0R469-R473 is a bit of magic, but I did not want to overengineer it. If someone has a better idea, let me know...

jrieken
jrieken approved these changes Nov 12, 2020
View reviewed changes
Copy link
Member

@jrieken jrieken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm - I also think that the check for command ids in the tree is a little hacky, esp when we copy that for SCM and the timeline. But maybe we come up with something before this starts to spread

@bpasero
Copy link
Member Author

bpasero commented Nov 12, 2020

@alexr00 please take a look at the JSDoc comment I adde to the command of TreeItem (8b0b052)

Please use vscode.open or vscode.diff as command IDs when the tree item is opening
something in the editor. Using these commands ensures that the resulting editor will
appear consistent with how other built-in trees open editors.

@bpasero bpasero merged commit 630e706 into master Nov 12, 2020
@bpasero bpasero deleted the ben/hijack-open-commands branch November 12, 2020 16:01
meganrogge pushed a commit that referenced this pull request Nov 18, 2020
@bpasero @meganrogge
Add open context to certain commands (#110475)
9a21ce4 
* editor commands - move API commands to workbench core

* rename EditorViewColumn => EditorGroupColumn

* mixin context to open commands

* address some feedback

* add comment
@github-actions github-actions bot locked and limited conversation to collaborators Dec 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@alexr00 alexr00 alexr00 approved these changes

@jrieken jrieken jrieken approved these changes

Assignees

@bpasero bpasero

Labels
None yet
Projects
None yet
Milestone
November 2020
Development

Successfully merging this pull request may close these issues.

TreeDataProvider: allow single click and double click to work the same way they do in the file exporer
3 participants
@bpasero @jrieken @alexr00