feat: replace newlines with crlf newlines when in a pty #185
Conversation
session.go
Outdated
| // with the extended data type set to stderr. Stderr may | ||
| // safely be read and written from a different goroutine than | ||
| // Read and Write respectively. | ||
| Stderr() io.ReadWriter |
There was a problem hiding this comment.
Shouldn't this be a plain io.Writer? I'm fairly certain you can't read data from stderr.
There was a problem hiding this comment.
IMHO it could, but it comes as a readwriter from ssh.Channel: https://pkg.go.dev/golang.org/x/crypto/ssh#Channel
which we embed on our session here:
Line 23 in db09465
That said, if I try to change it to writer only, it won't compile... so unfortunately I think there's no other way (besides maybe a new stderr method with another name).
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
|
|
||
| // NewPtyWriter creates a writer that handles when the session has a active | ||
| // PTY, replacing the \n with \r\n. | ||
| func NewPtyWriter(w io.Writer) io.Writer { |
There was a problem hiding this comment.
Do we need to export NewPtyWriter and NewPtyReadWriter?
There was a problem hiding this comment.
good question... I don't think we need to export, but I think it would be nicer if we do... whatever @belak decides is fine by me though.
Add session.Stderr test Fixes: 993008d ("refactor: keeping the same Stderr method")
|
@belak Could you take a look at this? |
Writing to the stdout of the session already handles replacing
\nwith\r\n, but the stderr didn't had the same treatment.Created another
io.Writerwrapper that can be used in both, and a "session.SafeStderr()` that returns it.PS: pretty sure "safe writer" is not a good name, also considered "crlf writer", let me know if you like one better than the other, or maybe yet another name