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

Consolidate local and remote terminal implementations #116467

Closed
Tyriar opened this issue Feb 11, 2021 · 4 comments · Fixed by #118252
Closed

Consolidate local and remote terminal implementations #116467

Tyriar opened this issue Feb 11, 2021 · 4 comments · Fixed by #118252
Assignees
@Tyriar
Labels
debt Code quality issues insiders-released Patch has been released in VS Code Insiders terminal Integrated terminal issues
Milestone
March 2021

Comments

@Tyriar
Copy link
Member

Tyriar commented Feb 11, 2021

Currently there's a fair bit of duplication
@Tyriar Tyriar added debt Code quality issues terminal Integrated terminal issues labels Feb 11, 2021
@Tyriar Tyriar added this to the February 2021 milestone Feb 11, 2021
@Tyriar Tyriar mentioned this issue Feb 11, 2021
Closed
@Tyriar
Copy link
Member Author

Tyriar commented Feb 11, 2021

This one is lower priority and also a little risky to do later in the iteration so it might get pushed to next debt week.

@Tyriar Tyriar mentioned this issue Feb 11, 2021
Merged
@Tyriar
Copy link
Member Author

Tyriar commented Feb 17, 2021

This is the plan, the topic local side is the current state of the local pty host (in PR), the bottom is what I want remote to look like:

Screen Shot 2021-02-17 at 12 46 54 PM

@Tyriar Tyriar modified the milestones: February 2021, March 2021 Feb 22, 2021
@meganrogge
Copy link
Contributor

meganrogge commented Feb 22, 2021 •
edited
Loading

@meganrogge remember to check if this breaks the remote case when remote starts using PtyService

@meganrogge
Copy link
Contributor

@meganrogge explored this and doesn't look like events are getting batched w the current throttleBy time of 5. #117268

@Tyriar Tyriar mentioned this issue Feb 24, 2021
Closed
Tyriar added a commit that referenced this issue Mar 4, 2021
@Tyriar
Move terminal instance service methods to local terminal service
3a1f4c4 
Part of #116467
@Tyriar Tyriar mentioned this issue Mar 4, 2021
Merged
Tyriar added a commit that referenced this issue Mar 5, 2021
@Tyriar
Move localPtyService to node
05ccfc4 
Part of #116467
Tyriar added a commit that referenced this issue Mar 5, 2021
@Tyriar
Progress towards sharing local and remote term impl
418b824 
Part of #116467
@Tyriar Tyriar mentioned this issue Mar 5, 2021
Merged
@kieferrm kieferrm mentioned this issue Mar 8, 2021
Closed
76 tasks
@github-actions github-actions bot locked and limited conversation to collaborators Apr 23, 2021
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 insiders-released Patch has been released in VS Code Insiders terminal Integrated terminal issues
Projects
None yet
Milestone
March 2021
Development

Successfully merging a pull request may close this issue.

Consolidate local and remote terminal implementations microsoft/vscode
3 participants
@Tyriar @meganrogge and others