-
Notifications
You must be signed in to change notification settings - Fork 5.9k
Use ptyHostService #3308
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
Use ptyHostService #3308
Conversation
Most of this was a straightforward replacement of our code with theirs but I also removed `getDefaultShellAndArgs` since it seems the reference implementation no longer does that either. Fixes coder#2276.
6d1da2c
to
c416e9b
Compare
Codecov Report
@@ Coverage Diff @@
## main #3308 +/- ##
=======================================
Coverage 58.95% 58.95%
=======================================
Files 35 35
Lines 1703 1703
Branches 374 374
=======================================
Hits 1004 1004
Misses 561 561
Partials 138 138 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice work, and win for maintainability! 🎉
Let's merge after e2e tests pass!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Woo! Awesome work. Excited to see this in action 🎉
Pulled locally, is failing with:
(Downloaded release package from CI) |
Tested locally, and terminal + persistence now works! Woohoo! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
Most of this was a straightforward replacement of our code with theirs
but I also removed
getDefaultShellAndArgs
since it seems the referenceimplementation no longer does that either.
Fixes #2276.