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

Allow not to disable echo when using pty mode #158

Merged
merged 3 commits into from
Jun 17, 2022
Merged

Allow not to disable echo when using pty mode #158

merged 3 commits into from
Jun 17, 2022

Conversation

SteffenDE
Copy link
Contributor

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
Copy link
Owner

saleyn commented Jun 16, 2022

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
Copy link
Contributor Author

SteffenDE commented Jun 17, 2022

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
Copy link
Owner

saleyn commented Jun 17, 2022

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

@saleyn saleyn merged commit 7f12101 into saleyn:master Jun 17, 2022
@saleyn
Copy link
Owner

saleyn commented Jun 17, 2022

Thank you for your contribution!

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.

2 participants