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

Save/Open local File/Folder commands should move to electron-browser #80959

Closed
bpasero opened this issue Sep 16, 2019 · 7 comments
Closed

Save/Open local File/Folder commands should move to electron-browser #80959

bpasero opened this issue Sep 16, 2019 · 7 comments
Assignees
Labels
debt Code quality issues simple-file-dialog Issues with simple file dialog

Comments

@bpasero
Copy link
Member

bpasero commented Sep 16, 2019

I notice quite a few actions/commands in browser/workspaceActions that talk about "local" file access (e.g for saving or opening). These currently live in the browser namespace but should move to electron-browser as we can never support them in the web.

This probably drags on to the file explorer where they also seem to be used and would require electron-browser there too.

List:

  • OpenLocalFileCommand
  • SaveLocalFileCommand
  • OpenLocalFolderCommand
  • OpenLocalFileFolderCommand

I am about to introduce a src/vs/workbench/electron-browser/actions/workspaceActions.ts where you can move them. Alternatively they could also be moved closer to where they are being used (explorer?)

//cc @isidorn

@bpasero bpasero added the debt Code quality issues label Sep 16, 2019
@isidorn
Copy link
Contributor

isidorn commented Sep 16, 2019

From the explorer land they are only used in the fileActions.contribution and we can easily add another fileActions.contribution which lives under electron-browser and those command would only be contributed when not in web. I can do that right now, so Alex can move those command to wherver is fitting. Having them in the explorer land does not fit really well since they are also included in the remoteFileDialog

@isidorn
Copy link
Contributor

isidorn commented Sep 16, 2019

Actually all these actions are remoteDialog specific, due to that I will introduce src/vs/workbench/services/dialogs/electron-browser/remoteFileActions.contribution.ts that will just register them.
@alexr00 I think that it then makes sense to move this actions close to the dialogs folder

@isidorn
Copy link
Contributor

isidorn commented Sep 16, 2019

Actually I put it here src/vs/workbench/contrib/remote/electron-browser/remote.contribution.ts
Feel free to move it around. I felt it belongs there most.

@alexr00
Copy link
Member

alexr00 commented Sep 17, 2019

@isidorn thank you for moving the commands. The file you put them in works well.

@alexr00 alexr00 added the simple-file-dialog Issues with simple file dialog label Sep 17, 2019
@alexr00 alexr00 added this to the September 2019 milestone Sep 17, 2019
@alexr00
Copy link
Member

alexr00 commented Sep 17, 2019

@bpasero the IDs and LABELS of those commands are used in the remoteFileDialog, which lives at src\vs\workbench\services\dialogs\browser\remoteFileDialog.ts. I can move the commands themselves, but the IDs and LABELS need to stay out of electron-browser.

@bpasero
Copy link
Member Author

bpasero commented Sep 17, 2019

@alexr00 I find workbench/common/actions.ts not a good place for declaring these commands. That module is more about laying the foundation for contributing actions in the wokbench, not so much for declaring available actions.

I do not fully understand why these things cannot move into the remoteFileDialog itself, they only seem to be used from there?

@bpasero bpasero reopened this Sep 17, 2019
@alexr00
Copy link
Member

alexr00 commented Sep 17, 2019

I didn't read carefully enough the first time and thought you wanted them in workspaceAction. Moved as suggested!

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues simple-file-dialog Issues with simple file dialog
Projects
None yet
Development

No branches or pull requests

3 participants