crossterm icon indicating copy to clipboard operation
crossterm copied to clipboard

resize cursor::MoveTo parameters to usize instead of u16

Open worldnamer opened this issue 4 years ago • 2 comments
trafficstars

Is your feature request related to a problem? Please describe. When using vectors to buffer screen input, Rust expects indexes to be provided as usize types. That means, if I have a buffer:Vec<Vec>, I can do something like buffer[row][column] ONLY if row and column are usize variables.

However, when using crossterm's MoveTo command, the parameters are u16. This leads to a lot of ugly code like the following:

  execute!(
    stdout(),
    cursor::MoveTo((self.input_buffer.len() + 3).try_into().unwrap(), self.buffer_line.try_into().unwrap()),
    style::Print(" "),
    cursor::MoveTo((self.input_buffer.len() + 3).try_into().unwrap(), self.buffer_line.try_into().unwrap())
  )?;

I believe this is on account of the Windows limitation around SetConsoleCursorPosition, which is how the Windows implementation sets the cursor position. This function limits cursor positions to a SHORT, which means u16.

That said, the function which calls SetConsoleCursorPosition is ScreenBufferCursor::move_to, which already does bounds checking on its inputs (which are currently i16, and can, I guess, potentially be negative?) It seems like it could easily check a usize to ensure its value falls between u16::MIN and u16::MAX.

Describe the solution you'd like It would be a delight if MoveTo took usize parameters. So many of the Rust std library functions return their values as a usize or expect their values as a usize, that it becomes super inconvenient to use MoveTo with those libraries. For instance, String::len returns a usize.

For the record, crossterm::terminal::size() would probably need to be modified as well to return a Result<(usize, usize)>.

Describe alternatives you've considered if any Macros might be another way to support this request, though I don't know if cursor::MoveTo(usizeToU16!(x), usizeToU16!(y)) is much better than the original. Maybe a macro that returns a MoveTo? Like, cursor::SensibleMoveTo!(x, y) that just calls try_into().unwrap()?

Additional context The presence of this library is pretty awesome. Thank you for writing it.

worldnamer avatar Jul 07 '21 07:07 worldnamer

I don't have any objections to it really. I don't recall the reason why u16 was introduced in the first place. 'cursor set', 'cursor get', 'get a terminal size', 'set terminal size' have to be updated then. u16 is kinda the standard used throughout the library.

TimonPost avatar Jul 09 '21 12:07 TimonPost

More generally, the u16 in crossterm are really painful for all the conversions they imply.

In a library I maintain, which uses crossterm, I've started migrating everything from u16 to usize, with compatibility given by prototypes like this one:

    pub fn scrollbar<U>(
        &self,
        scroll: U, // number of lines hidden on top
        content_height: U,
    ) -> Option<(u16, u16)>
    where U: Into<usize>
    {

(so that code passing u16 still compiles)

Canop avatar Oct 02 '21 13:10 Canop