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

feat: use real pty #197

Merged
merged 38 commits into from
Jan 18, 2024
Merged

feat: use real pty #197

merged 38 commits into from
Jan 18, 2024

Conversation

caarlos0
Copy link
Member

@caarlos0 caarlos0 commented Dec 4, 2023 •
edited
Loading

This allows to allocate a real PTY for the session, allowing to do things like running vim within a SSH session.

depends on charmbracelet/bubbletea#879
depends on charmbracelet/ssh#8
closes #196

@caarlos0
feat: use pty
0b36a69 
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0 caarlos0 added the enhancement New feature or request label Dec 4, 2023
@caarlos0 caarlos0 self-assigned this Dec 4, 2023
@caarlos0
fix: deps
aa0bb66 
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@codecov Codecov
Copy link

codecov bot commented Dec 4, 2023 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d2fab68) 76.40% compared to head (20ca97a) 76.40%.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main     #197   +/-   ##
=======================================
  Coverage   76.40%   76.40%           
=======================================
  Files          17       17           
  Lines         907      907           
=======================================
  Hits          693      693           
  Misses        160      160           
  Partials       54       54

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@caarlos0 caarlos0 mentioned this pull request Dec 4, 2023
Closed
@caarlos0
fix: deps
fc89255 
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
aymanbagabas
aymanbagabas reviewed Dec 4, 2023
View reviewed changes
Copy link
Member

@aymanbagabas aymanbagabas left a comment

Choose a reason for hiding this comment

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

If we're going to do that, we have to remove the ssh PtyWriter hack, and detect the color profile from the PTY https://github.com/charmbracelet/wish/pull/197/files#diff-79d3f1c4cf631bd3e71a422f4d73666477d1caa5b44e965c98d80e6551f4f8cfR93

@caarlos0
Copy link
Member Author

caarlos0 commented Dec 7, 2023

If we're going to do that, we have to remove the ssh PtyWriter hack, and detect the color profile from the PTY https://github.com/charmbracelet/wish/pull/197/files#diff-79d3f1c4cf631bd3e71a422f4d73666477d1caa5b44e965c98d80e6551f4f8cfR93

Can be done in another PR, as its not strictly related

@caarlos0 caarlos0 changed the title feat: use real pty feat!: use real pty Dec 8, 2023
@caarlos0
Merge remote-tracking branch 'origin/main' into using-pty
c6e19fc 
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix: go mod tidy
a70d0bd 
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
improvements
fd4337e 
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
Merge remote-tracking branch 'origin/main' into using-pty
488fd2f 
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
Copy link
Member Author

caarlos0 commented Jan 4, 2024

still pending changes on ssh cc/ @aymanbagabas

aymanbagabas
aymanbagabas reviewed Jan 4, 2024
View reviewed changes
bubbletea/tea.go Outdated Show resolved Hide resolved
caarlos0 and others added 2 commits January 4, 2024 21:55
@caarlos0
Merge remote-tracking branch 'origin/main' into using-pty
c9ad8ee 
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0 @aymanbagabas
fix: pty
83cc777 
Co-authored-by: Ayman Bagabas <ayman.bagabas@gmail.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
caarlos0 and others added 5 commits January 4, 2024 22:10
@caarlos0
fix: better diff
60e8254
@caarlos0
chore: typo
2cf714d
@caarlos0
fix: godocs
59c5884
@caarlos0
fix: allocate pty on macos
9ab015a 
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0 @aymanbagabas
fix: improvements
50a706e 
based on #219

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
Co-authored-by: Ayman Bagabas <ayman.bagabas@gmail.com>
@caarlos0
chore: godoc
411f301
@caarlos0
Merge remote-tracking branch 'origin/main' into using-pty
9d68adf 
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
aymanbagabas
aymanbagabas reviewed Jan 9, 2024
View reviewed changes
bubbletea/tea.go Outdated Show resolved Hide resolved
bubbletea/tea.go Outdated Show resolved Hide resolved
bubbletea/tea_other.go Outdated Show resolved Hide resolved
bubbletea/tea_unix.go Outdated Show resolved Hide resolved
caarlos0 and others added 8 commits January 9, 2024 12:54
@caarlos0 @aymanbagabas
fix: review
47074ac 
Co-authored-by: Ayman Bagabas <ayman.bagabas@gmail.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix: example
5e7f41a 
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix: tea program handler
4c7fb4f 
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix: examples
0431178
@caarlos0
Merge remote-tracking branch 'origin/main' into using-pty
aed4f59 
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
Merge remote-tracking branch 'origin/main' into using-pty
c40b251
@caarlos0
refactor: improve p!=nil handling
d7ee1ec 
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@aymanbagabas
fix: ensure session envs are available to renderer (#223)
c0d4269
aymanbagabas
aymanbagabas approved these changes Jan 9, 2024
View reviewed changes
bubbletea/tea.go Outdated Show resolved Hide resolved
@caarlos0 caarlos0 mentioned this pull request Jan 17, 2024
Closed
1 task
@caarlos0
Merge remote-tracking branch 'origin/main' into using-pty
c9668e0 
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix: dep
91c322b 
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
Copy link
Member Author

anyone against this not being a breaking change anymore?

@aymanbagabas
Copy link
Member

anyone against this not being a breaking change anymore?

LGTM

@caarlos0 caarlos0 changed the title feat!: use real pty feat: use real pty Jan 18, 2024
@caarlos0
fix: update dep
12a7a6a 
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
chore: go mod tidy
731c66e 
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
chore: update example
20ca97a
@caarlos0 caarlos0 merged commit 7f44d6d into main Jan 18, 2024
14 checks passed
@caarlos0 caarlos0 deleted the using-pty branch January 18, 2024 18:14
@caarlos0 caarlos0 mentioned this pull request Jan 18, 2024
Closed
@matthewp
Copy link

@caarlos0 Do you plan on doing a release for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aymanbagabas aymanbagabas aymanbagabas approved these changes

Assignees

@caarlos0 caarlos0

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bubbletea ExecProcess within a wish session
3 participants
@caarlos0 @aymanbagabas @matthewp