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.

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

feat: add support for Browser Terminal #1673

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

hunnywar
Copy link
Contributor

@hunnywar hunnywar commented Dec 30, 2024

feat: add support for Browser Terminal

Description

this allows for web terminal or terminal in browser

  • This change requires a documentation update
  • I have made corresponding changes to the documentation

Related Issue(s)

This PR addresses issue #X

Screenshots

image

Notes

for testing purposes

replace https://download.daytona.io/daytona/tools/get-ttyd.sh

with

https://raw.githubusercontent.com/daytonaio/daytona/a0b9e920331a7381079afacbd2c1e440aecdcff8/hack/get-ttyd.sh

@hunnywar hunnywar requested review from a team as code owners December 30, 2024 16:05
@mojafa
Copy link
Collaborator

mojafa commented Jan 7, 2025

@hunnywar running go run ./cmd/daytona/main.go ide and i'm not seeing Browser Terminal.
Screenshot 2025-01-07 at 15 46 07

@mojafa
Copy link
Collaborator

mojafa commented Jan 7, 2025

nvm found it:
Screenshot 2025-01-07 at 16 06 46

@mojafa
Copy link
Collaborator

mojafa commented Jan 7, 2025

@Tpuljak it works well.

had to replace https://download.daytona.io/daytona/get-ttyd.sh
with https://raw.githubusercontent.com/daytonaio/daytona/27b57bb3917160a9dd4846d50a8fb0e1f44c05e5/hack/get-ttyd.sh
in daytona/pkg/ide/browser-terminal.go
Screenshot 2025-01-07 at 16 14 48

@hunnywar
Copy link
Contributor Author

hunnywar commented Jan 7, 2025

image
It is already written


err = startServerCommand.Run()
if err != nil {
log.Fatal(err)
Copy link
Member

Choose a reason for hiding this comment

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

Screenshot 2025-01-08 at 09 08 15

When a user cancels the process (ctrl+c), the command should not exit with a fatal error. I believe it originates from here.

Make sure that the command exits gracefully.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Tpuljak I guess VS code Browser shares the same fate
image
should I also refactor that one ?

Copy link
Member

Choose a reason for hiding this comment

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

@hunnywar yes, please fix that here as well

pkg/ide/browser-terminal.go Outdated Show resolved Hide resolved
pkg/ide/browser-terminal.go Outdated Show resolved Hide resolved
Signed-off-by: hunnywar <[email protected]>
Signed-off-by: hunnywar <[email protected]>
Signed-off-by: hunnywar <[email protected]>
@hunnywar hunnywar force-pushed the feat/terminal-browser branch from 27b57bb to 38580b4 Compare January 8, 2025 13:32
Signed-off-by: hunnywar <[email protected]>
Signed-off-by: hunnywar <[email protected]>
Signed-off-by: hunnywar <[email protected]>
Signed-off-by: hunnywar <[email protected]>
Signed-off-by: hunnywar <[email protected]>
@hunnywar
Copy link
Contributor Author

hunnywar commented Jan 8, 2025

@Tpuljak this test is flaky
image

@Tpuljak
Copy link
Member

Tpuljak commented Jan 9, 2025

@hunnywar let's get the VS Code insiders merged first and then come back to rebase this one.

Regarding the flaky test, I'll take a look at it. You can ignore it here for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants