Add Pty.TerminalModes

[elfgzp]

No description provided.

[shazow]

Looks reasonable, but I've never used this feature so a sanity check on ergonomics from someone else would be great. :)

[belak]

This looks like a really solid starting point. Unless I'm misreading something though, this doesn't allow for changing terminal modes at runtime.

As an example, things like disabling ECHO and ICANON to enter raw mode can be changed to allow for password entry without displaying the characters.

I'm not 100% sure how to solve this, especially with an ergonomic API, so I'd be open to any suggestions.

[belak]

As far as I can tell, this looks like a great first step - also, from reading the spec, I believe the only terminal modes that can be sent at runtime are xon/xoff (https://tools.ietf.org/html/rfc4254#section-6.8) which are handled in a separate request and should therefore be fine (since we'd implicitly deny them).

I left a number of comments, though most of them are stylistic changes.

[belak]

This looks like it needs a gofmt

[elfgzp]

Oh sorry, I forgot it. I will modify this.

[belak]

The description on how terminal modes are actually passed can be found here: https://tools.ietf.org/html/rfc4254#section-8

[elfgzp]

I'm not sure if I understand what you mean. Do you mean to set terminal modes to []byte type?

[belak]

These are already implicitly returned because of the named params. Please either switch back to the implicit return or remove the variable names from the function definition.

[elfgzp]

👌

[belak]

This should be enough for now, but we may want to have an example or a convenience function that will handle this properly. Setting TerminalModes is not fun. This is just an example: https://github.com/golang/crypto/blob/master/ssh/terminal/util.go#L37

[elfgzp]

Did you mean like this ?

func ApplyTerminalModes(fd int, terminalModes ssh.TerminalModes) bool {
	termios, err := unix.IoctlGetTermios(fd, unix.TCGETS)
	fmt.Println(termios, err)
	for opcode, val := range terminalModes {
		switch opcode {
		case ssh.TTY_OP_ISPEED:
			termios.TTY_OP_OSPEED = val
		case ssh.TTY_OP_OSPEED:
			termios.TTY_OP_OSPEED = val
		
		// case char
		case ssh.VINTR:
			termios.Cc[ssh.VINTR] = val
		case ssh.VQUIT:
			termios.Cc[ssh.VQUIT] = val
		//...
		//case mode
		case ssh.ECHO:
			if val {
				termios.Lflag |= ssh.ECHO
			} else {
				termios.Lflag &= ~ssh.ECHO
			}

		default
			// Others modes ...
		}

	}

	if err := unix.IoctlSetTermios(fd, unix.TCSETS, termios); err != nil {
		return nil, err
	}
	return true
}

This is the reference code https://github.com/openssh/openssh-portable/blob/1a7217ac063e48cf0082895aeee81ed2b8a57191/ttymodes.c#L397, the syntax may not be correct.

[belak]

This actually handles 2 different conditions - I'd argue len(modes) < 1 || modes[0] == ttyOPEND are success, but len(modes) < 5 is a failure.

[belak]

One other note: I'm not sure what https://tools.ietf.org/html/rfc4254#section-8 implies about opcodes 160-255. I think it means that when reading in the opcodes, and you get 160-255, it should end processing. It also may imply that items should be read in item by item... since I don't know if clients are required to send a Val with opcodes 160-255.

[elfgzp]

Thanks for reviewing my code. My English may not be very good. I'm actually not too familiar with terminal modes. I want to pass it to my next Pty request in my project.
About opcodes 160-255, I think I can refer to https://github.com/openssh/openssh-portable/blob/master/ttymodes.c#L421. Ignore opcodes 160-255 passed by the client.