Skip to content

Add ability to spawn a tty using a system command#8

Open
ConnorRigby wants to merge 1 commit into
jjcarstens:mainfrom
ConnorRigby:main
Open

Add ability to spawn a tty using a system command#8
ConnorRigby wants to merge 1 commit into
jjcarstens:mainfrom
ConnorRigby:main

Conversation

@ConnorRigby

Copy link
Copy Markdown

Allows you too do something like:

ExTTY.start_link(handler: self(), type: :muontrap, shell_opts: [[exec: System.find_executable("bash"), args: []]])

@jjcarstens jjcarstens left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This is so cool!

Comment thread lib/extty.ex
{Elixir.IEx, :start, opts}
end

defp shell_spawner(%{type: :muontrap, shell_opts: opts}) do

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
defp shell_spawner(%{type: :muontrap, shell_opts: opts}) do
defp shell_spawner(%{type: :system, shell_opts: opts}) do

MuonTrap is an implementation detail. What if we generalize this to System?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

How about letting the user pass a :custom shell spawner that calls an arbitrary function?

I'd be for adding an example to the docs if that's direction that you go since I wouldn't have expected calling System.cmd to work well with this. I'm thrilled that it does, though.

Comment thread lib/extty.ex
args = Keyword.fetch!(opts, :args)
gl = Process.group_leader()

spawn(fn ->

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
spawn(fn ->
spawn_link(fn ->

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