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

fix: create a new pty for exec.Cmd #230

Merged
merged 12 commits into from
Jan 26, 2024
Merged

fix: create a new pty for exec.Cmd #230

merged 12 commits into from
Jan 26, 2024

Conversation

caarlos0
Copy link
Member

dirty fix to "inappropriate ioctl for device" on macOS.

It seems that macOS kills the slave pty once exec.Cmd finishes... so we need to create a new PTY just to run said program in order to avoid that problem.

Need to test, but this might make things possible on windows, too.

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0 caarlos0 added the enhancement New feature or request label Jan 23, 2024
@caarlos0 caarlos0 self-assigned this Jan 23, 2024
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
Copy link

codecov bot commented Jan 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (pty-command@1681ea4). Click here to learn what that means.

Additional details and impacted files
@@              Coverage Diff               @@
##             pty-command     #230   +/-   ##
==============================================
  Coverage               ?   77.23%           
==============================================
  Files                  ?       19           
  Lines                  ?      940           
  Branches               ?        0           
==============================================
  Hits                   ?      726           
  Misses                 ?      160           
  Partials               ?       54           

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

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
Copy link
Member Author

It "kinda" works on windows, but when the pty dies, so dies the parent process (the wish server).

Not sure if it is fixable...

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0 caarlos0 merged commit 98030fa into pty-command Jan 26, 2024
14 checks passed
@caarlos0 caarlos0 deleted the pty-command-2 branch January 26, 2024 18:06
caarlos0 added a commit that referenced this pull request Jan 30, 2024
* feat: add wish.Command and wish.Cmd

The wish-exec example with vim worked because neovim was not using
STDERR.

Bubbletea doesn't have a concept of stdout and stderr, just output, so
`tea.ExecProcess` sets the `exec.Cmd` stderr to `os.Stderr`.

This would fail for bash, for instance.

This also introduces a `wish.Cmd` type and a `wish.Command`
function to properly set up a `wish.Cmd` based on `ssh.Session` (and
optionally a Pty), which can then be used with `tea.Exec`.

Finally, it adds to the wish-exec example, including the `s` key to run
a shell (bash).

closes #228

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

* feat: allow to set dir and env

* refactory: move things around a bit

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

* fix: loop

* test: add some tests

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

* test: more tests

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

* test: more tests

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

* fix: improve tests

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

* test: windows

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

* fix: review

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

* fix: improve cmd

* fix: create a new pty for exec.Cmd (#230)

* fix: create a new pty for exec.Cmd

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

* fix: improvements

* fix: test

* fix: windows

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

* fix: unneeded err

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

* fix

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

* fix: resize && race

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

* fix: windows/linux

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

* test: ignore on windows for now

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

* fix: sync

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

* test: hammer

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

* chore: import

---------

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

* fix: update ssh

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

---------

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant