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 node/electron-browser in workbench/contrib/terminal #69115

Closed
bpasero opened this issue Feb 21, 2019 · 1 comment · Fixed by #69816
Closed

Revisit need for node/electron-browser in workbench/contrib/terminal #69115

bpasero opened this issue Feb 21, 2019 · 1 comment · Fixed by #69816
Assignees
@Tyriar
Labels
debt Code quality issues terminal Integrated terminal issues
Milestone
March 2019

Comments

@bpasero
Copy link
Member

bpasero commented Feb 21, 2019

Refs #68302

I am not sure why src/vs/workbench/contrib/terminal 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 mentioned this issue Feb 21, 2019
Closed
17 tasks
@Tyriar Tyriar added this to the March 2019 milestone Mar 5, 2019
@Tyriar Tyriar added the terminal Integrated terminal issues label Mar 5, 2019
Tyriar added a commit that referenced this issue Mar 5, 2019
@Tyriar
Move terminalLinkHandler, electron-browser->browser
fe169ce 
Part of #69115
@Tyriar Tyriar mentioned this issue Mar 5, 2019
Merged
Tyriar added a commit that referenced this issue Mar 5, 2019
@Tyriar
Move terminalEnvironment, node->common
badedf5 
Part of #69115
Tyriar added a commit that referenced this issue Mar 5, 2019
@Tyriar
Move terminalActions, electron-browser->browser
5b7bdac 
Part of #69115
Tyriar added a commit that referenced this issue Mar 5, 2019
@Tyriar
Move terminalPanel, electron-browser->browser
3760c27 
Part of #69115
Tyriar added a commit that referenced this issue Mar 5, 2019
@Tyriar
Move TerminalConfigHelper, electron-browser->browser
1d1dbdc 
Part of #69115
Tyriar added a commit that referenced this issue Mar 5, 2019
@Tyriar
Move terminal CSS and images to browser
b5dcee0 
Part of #69115
Tyriar added a commit that referenced this issue Mar 5, 2019
@Tyriar
Move terminalCommandTracker, node->browser
00f5bdb 
Part of #69115
Tyriar added a commit that referenced this issue Mar 5, 2019
@Tyriar
Move terminalInstance, electron-browser->browser
b5a06eb 
Part of #69115
@Tyriar
Copy link
Member

Tyriar commented Mar 5, 2019

@bpasero I've moved heaps of stuff up to common/browser in #69816, electron-browser/TerminalService and the new electron-browser/TerminalInstanceService are both tiny (~150, 50 lines) and the files in node/ are responsible for interfacing with with processes are fairly small as well. This was a fun refactor 😃

I'll merge it in tomorrow if tests pass.

@kieferrm kieferrm mentioned this issue Mar 11, 2019
Closed
26 tasks
@vscodebot vscodebot bot locked and limited conversation to collaborators Apr 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@Tyriar Tyriar

Labels
debt Code quality issues terminal Integrated terminal issues
Projects
None yet
Milestone
March 2019
Development

Successfully merging a pull request may close this issue.

Move parts of contrib/terminal to browser layer microsoft/vscode
2 participants
@bpasero @Tyriar