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

Revisit need for electron-browser in workbench/contrib/codeEditor #69102

Closed
bpasero opened this issue Feb 21, 2019 · 5 comments
Closed

Revisit need for electron-browser in workbench/contrib/codeEditor #69102

bpasero opened this issue Feb 21, 2019 · 5 comments
Assignees
@alexdima
Labels
debt Code quality issues
Milestone
February 2019

Comments

@bpasero
Copy link
Member

bpasero commented Feb 21, 2019 •
edited

Refs #68302

I am not sure why src/vs/workbench/contrib/codeEditor has to have most of its files in electron-browser land. Please consider lifting this to browser if possible.

If there is something blocking this task, speak up and we need to think about introducing new services as needed.
@bpasero bpasero added the debt Code quality issues label Feb 21, 2019
@bpasero bpasero changed the title Revisit need for electron-browser in codeeditor Revisit need for electron-browser in workbench/contrib/codeEditor Feb 21, 2019
@bpasero bpasero mentioned this issue Feb 21, 2019
Closed
17 tasks
alexdima added a commit that referenced this issue Feb 21, 2019
@alexdima
Move most code editor contributions to /browser/ (#69102)
6955375
alexdima added a commit that referenced this issue Feb 21, 2019
@alexdima
Move inspectKeybindings to /browser/ (#69102)
5406a9f
@alexdima alexdima added this to the February 2019 milestone Feb 21, 2019
@alexdima
Copy link
Member

@bpasero I've moved everything except some code which forces an editor repaint when we receive an vscode:osResume ipc event. Not sure where that code should move now?

alexdima added a commit that referenced this issue Feb 21, 2019
@alexdima
Move inspectTMScropes to /browser/ (#69102)
5f9ca13
@bpasero
Copy link
Member Author

bpasero commented Feb 21, 2019

@alexandrudima thanks for the prompt move, I agree the remaining stuff seems very specific to fixing that issue for us so we can leave it like this for now.

Would it make sense to have 2 contribution files (codeEditorContribution.ts) so that we can split up between browser contributions and electron-browser?

@alexdima
Copy link
Member

alexdima commented Feb 21, 2019 •
edited

Would it make sense to have 2 contribution files (codeEditorContribution.ts) so that we can split up between browser contributions and electron-browser?

👍. pushed.

alexdima added a commit that referenced this issue Feb 21, 2019
@alexdima
Split the code editor contributions (#69102)
58a3231
sandy081 pushed a commit to vldmrkl/vscode that referenced this issue Feb 22, 2019
@alexdima @sandy081
Move most code editor contributions to /browser/ (microsoft#69102)
b876d2f
sandy081 pushed a commit to vldmrkl/vscode that referenced this issue Feb 22, 2019
@alexdima @sandy081
Move inspectKeybindings to /browser/ (microsoft#69102)
3ff6973
sandy081 pushed a commit to vldmrkl/vscode that referenced this issue Feb 22, 2019
@alexdima @sandy081
Move inspectTMScropes to /browser/ (microsoft#69102)
6ee4777
sandy081 pushed a commit to vldmrkl/vscode that referenced this issue Feb 22, 2019
@alexdima @sandy081
Split the code editor contributions (microsoft#69102)
492045a
@bpasero bpasero reopened this Feb 23, 2019
@bpasero
Copy link
Member Author

bpasero commented Feb 23, 2019

@alexandrudima just a small nit, there is a process.nextTick usage still which unfortunately our layering rules does not capture.

@bpasero
Copy link
Member Author

bpasero commented Feb 23, 2019

Nevermind, I can change this in debt week. My idea is to introduce a vs/base/common/process that simply delegates to platform.setImmediate(). I would then also use that module from vs/base/common/path.

@bpasero bpasero closed this as completed Feb 23, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Apr 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@alexdima alexdima

Labels
debt Code quality issues
Projects
None yet
Milestone
February 2019
Development

No branches or pull requests
2 participants
@bpasero @alexdima