Skip to content

Conversation

code-asher
Copy link
Member

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 #2276.

@code-asher code-asher requested a review from a team as a code owner May 7, 2021 16:27
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.
@codecov
Copy link

codecov bot commented May 7, 2021

Codecov Report

Merging #3308 (19668b2) into main (b21a9af) will not change coverage.
The diff coverage is n/a.

❗ Current head 19668b2 differs from pull request most recent head bdb230b. Consider uploading reports for the commit bdb230b to get more accurate results
Impacted file tree graph

@@           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.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b21a9af...bdb230b. Read the comment docs.

Copy link

@oxy oxy left a 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!

@jsjoeio jsjoeio added the enhancement Some improvement that isn't a feature label May 7, 2021
@jsjoeio jsjoeio modified the milestones: v3.9.5, v3.9.4 May 7, 2021
Copy link
Contributor

@jsjoeio jsjoeio left a 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 🎉

@oxy oxy self-requested a review May 7, 2021 18:16
@oxy
Copy link

oxy commented May 7, 2021

Pulled locally, is failing with:

[IPC Library: Pty Host] Error: ENOENT: no such file or directory, open '/var/home/oxy/Downloads/code-server-3.9.3-linux-amd64/lib/vscode/out/vs/platform/terminal/node/ptyHostMain.js'

(Downloaded release package from CI)

@oxy
Copy link

oxy commented May 7, 2021

Tested locally, and terminal + persistence now works! Woohoo!

Copy link

@oxy oxy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@code-asher code-asher merged commit 5d5ecdc into coder:main May 7, 2021
@code-asher code-asher deleted the pty-host-service branch May 7, 2021 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Some improvement that isn't a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Persistant terminal session on tab refresh
3 participants