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

Add from_file_descriptors() to tty::unix #7940

Merged
merged 2 commits into from May 1, 2024

Conversation

someone13574
Copy link
Contributor

Adds a function to create a Pty handle from file descriptors opened outside of Alacritty. This does not break backward compatibility.

@someone13574 someone13574 changed the title Add from_file_descriptors to tty::unix Add from_file_descriptors() to tty::unix Apr 30, 2024
Copy link
Contributor

@nixpulvis nixpulvis left a comment

Choose a reason for hiding this comment

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

Is there a reason for this change? A use case for a consumer application?

alacritty/Cargo.toml Outdated Show resolved Hide resolved
@someone13574
Copy link
Contributor Author

someone13574 commented Apr 30, 2024

Is there a reason for this change? A use case for a consumer application?

I'm working on Flatpak support for Zed, which uses alacritty_terminal for it's builtin terminal, but in order to properly spawn the shell process on the host system one needs to pass the file descriptors of the command been spawned to the --forward-fd argument flatpak-spawn. This doesn't work however as alacritty doesn't expose the file descriptors until we have already passed the shell command to use, thus the need open the PTY ourselves so the fd's are available.

I figured the least intrusive way of doing this was just introducing a new function which lets you pass your own file descriptors.

Copy link
Member

@chrisduerr chrisduerr left a comment

Choose a reason for hiding this comment

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

Seems simple enough and the stuff you have to do outside is also trivial (openpty).

So this solution makes more sense than basically exposing a function that is the same as openpty just with some spawn call attached.

alacritty_terminal/src/tty/unix.rs Outdated Show resolved Hide resolved
alacritty_terminal/src/tty/unix.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@nixpulvis nixpulvis left a comment

Choose a reason for hiding this comment

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

😄

Just letting @chrisduerr make the final review.

Copy link
Member

@chrisduerr chrisduerr left a comment

Choose a reason for hiding this comment

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

Another tiny nitpick. Doubt I can find anything else.

alacritty_terminal/CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Christian Duerr <contact@christianduerr.com>
@chrisduerr chrisduerr merged commit ed3fac8 into alacritty:master May 1, 2024
5 checks passed
@chrisduerr
Copy link
Member

@someone13574 Do you need this released?

@someone13574
Copy link
Contributor Author

@someone13574 Do you need this released?

Yes, that would be awesome.

@someone13574 someone13574 deleted the add-from-file-descriptors branch May 1, 2024 05:29
@chrisduerr
Copy link
Member

No promises on the ETA, but I'll try and get a release out this week.

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

Successfully merging this pull request may close these issues.

None yet

3 participants