saleyn / erlexec

Execute and control OS processes from Erlang/OTP
https://hexdocs.pm/erlexec/readme.html
Other
525 stars 139 forks source link

Allow not to disable echo when using pty mode #158

Closed SteffenDE closed 2 years ago

SteffenDE commented 2 years ago

Hi there,

I'm currently working on something that uses erlexec to run interactive terminal sessions. For this, I needed a way to skip disabling the echo mode when using a pseudo terminal. I'm not quite sure if this is the right way to do this or if there should be even more control over the pty settings, but this works well for me and I sadly do have neither the Erlang nor the C++ experience to implement something more sophisticated than this.

Also, I noticed that some tests were failing for me until I changed the line io_lib:printable_list(Cmd) to io_lib:printable_unicode_list(Cmd) that is completely unrelated to my changes.

What are your thoughts on this?

saleyn commented 2 years ago

This sounds good. Though I would prefer to rename the new parameter to pty_echo = true | false with false being the default. Do you mind updating the PR?

SteffenDE commented 2 years ago

Yes you're right. I initially called it pty_echo, but then thought that actually we're not "enabling" echo but rather not disable it in the first place. My naming choice was rather awkward still. I don't know if there are systems where echo has to be manually enabled, therefore the pty_echo name being misleading. Maybe there could be room for improvement by allowing something like: {pty_modes, [{echo, 1}, {echoe, 1}, {echok, 1}]}, see also https://www.erlang.org/doc/man/ssh_connection.html#type-pty_ch_msg and https://datatracker.ietf.org/doc/html/rfc4254#section-8

saleyn commented 2 years ago

I agree that it would be great to support all modes mentioned in section-8 of the RFC4254.

saleyn commented 2 years ago

Thank you for your contribution!