Add line field to Termios struct
#1805
Conversation
tertsdiepraam
force-pushed
the
termios-line
branch
2 times, most recently
from
376b677
to
e83ab05
Compare
src/sys/termios.rs
Outdated
| @@ -181,6 +181,16 @@ pub struct Termios { | |||
| pub local_flags: LocalFlags, | |||
| /// Control characters (see `termios.c_cc` documentation) | |||
| pub control_chars: [libc::cc_t; NCCS], | |||
| /// Line number (see `termios.c_line` documentation) | |||
There was a problem hiding this comment.
c_line is described on Linux as "line discipline", not "line number". I'm guessing it's the same for other platforms.
There's an enumeration of the disciplines on Linux here. I don't think we need to add them to Nix necessarily, but I do see that they are also missing from https://github.com/rust-lang/libc; you might consider adding them there.
There was a problem hiding this comment.
Ah of course. I'll update the PR. Shall I add the possible values to the docstring instead of making an enum?
There was a problem hiding this comment.
And do you think the field should be called line_discipline given that the other fields also have more descriptive names than libc?
There was a problem hiding this comment.
I do think line_discipline is a better name.
I don't have a strong preference between using a numeric type vs. an enum. If you want to go the enum route, we'd want to derive our enum values from the libc definition, which we do using the libc_bitflags! macro. This would necessitate adding them to libc before introducing them here. I'll leave that choice up to you.
If you're sticking with a numeric type, I don't see a need to enumerate them fully in Nix's documentation. I think referring users to their platform-specific documentation is fine.
There was a problem hiding this comment.
I'll stick with numbers for now. In existing stty implementations the number is also used directly. Thank you for your feedback!
tertsdiepraam
force-pushed
the
termios-line
branch
2 times, most recently
from
326ae90
to
2b91f51
Compare
CHANGELOG.md
Outdated
| @@ -6,6 +6,9 @@ This project adheres to [Semantic Versioning](https://semver.org/). | |||
| ## [Unreleased] - ReleaseDate | |||
| ### Added | |||
|
|
|||
| - Added `line` field to `Termios` on Linux, Android and Haiku | |||
There was a problem hiding this comment.
Can you update this to line_discipline?
tertsdiepraam
force-pushed
the
termios-line
branch
from
2b91f51
to
810f74a
Compare
Fixes #1802
I have to admit I'm not really sure how to test this properly, so if that's necessary I require some help :)