Added accounting for terminal size

[BjornTheProgrammer]

Fixes #64 and #79, and is based upon work in #73, but just refined.

Adds one external API called set_size to modify the number of rows displayed at once for select and multiselect.

By default, this ensures that there are not more than the maximum possible rows displayed at once.

[fadeevab]

@BjornTheProgrammer Thanks! I'll check it out later. As of now, apply rustfmt over your changes (Alt+Shift+F in vscode), I can see TABs and some formatting inconsistencies.

[BjornTheProgrammer]

Whoops, should be fixed now. I ran cargo fmt, which should be the same as VSCode's.

[BjornTheProgrammer]

I realized that the test used the wrong function name, I've updated that.

[furbyhaxx]

Just tested your changes and you should check if termsize::get() returns a zero size or if you can subtract as for the RustRover debug terminal it returns 0,0 and panics because it tries to subtract with overflow.

impl Default for TermSize {
    fn default() -> Self {
        let mut window_size = usize::MAX;

        if let Some(termsize) = termsize::get() {
            window_size = termsize.rows as usize - 3; // <-- here
            // something like this:
            window_size = (termsize.rows as usize).checked_sub(3).unwrap_or(termsize.rows);

        }

        Self {
            window_size,
            window_pos: 0,
        }
    }
}

[BjornTheProgrammer]

Makes perfect sense! I'll implement the change as requested. Probably should have done this to begin with 😅️.

[BjornTheProgrammer]

Should be implemented now, please let me know if you have any other issues!

[furbyhaxx]

termsize does not work for SSH terminals, like RustRover trough Jetbrains Gateway.
I made a fix will submit a PR for termsize which fixes this, but until then we could directly integrate a workaround for this case:

extern crate libc;

use std::io::IsTerminal;

use std::ffi::{c_ushort, CString};

use self::{
    super::Size,
    libc::{ioctl, O_RDONLY, STDOUT_FILENO, TIOCGWINSZ},
};

/// A representation of the size of the current terminal
#[repr(C)]
#[derive(Debug)]
pub struct UnixSize {
    /// number of rows
    pub rows: c_ushort,
    /// number of columns
    pub cols: c_ushort,
    x: c_ushort,
    y: c_ushort,
}


/// Workaround for SSH terminal size
pub fn get_term_size() -> Option<termsize::Size> {
    if cfg!(target_os = "windows") {
        termsize::get()
    } else {
        _get_unix_termsize()
    }
}

/// Gets the current terminal size
fn _get_unix_termsize() -> Option<termsize::Size> {
    // http://rosettacode.org/wiki/Terminal_control/Dimensions#Library:_BSD_libc
    if !std::io::stdout().is_terminal() {
        return None;
    }
    let mut us = UnixSize {
        rows: 0,
        cols: 0,
        x: 0,
        y: 0,
    };

    let fd = if let Ok(ssh_term) = std::env::var("SSH_TTY") {
        // Convert path to a C-compatible string
        let c_path = CString::new(ssh_term).expect("Failed to convert path to CString");

        // Open the terminal device
        let fd = unsafe { libc::open(c_path.as_ptr(), O_RDONLY) };
        if fd < 0 {
            return None; // Failed to open the terminal device
        }

        fd
    } else {
        STDOUT_FILENO
    };

    let r = unsafe { ioctl(fd, TIOCGWINSZ, &mut us) };

    if r == 0 {
        Some(termsize::Size {
            rows: us.rows,
            cols: us.cols,
        })
    } else {
        None
    }
}

Here we can just call get_term_size instead of termsize::get() but it introduces a new dependency (libc).

EDIT: Submitted the PR to termsize: PR #24

[BjornTheProgrammer]

Thank you for submitting the PR!

I'm more than willing to add libc as a temporary fix, but it really is up to whether @fadeevab wants it. If you want to use this patch now, without waiting for your termsize patch to be merged, you could probably override both dependencies in the cargo.toml

[fadeevab]

Hey, folks! You're still here 😄
If you want to bring a libc dependency, do it in a cross-platform way: the code should compile on Windows.

Other than that, I think the "size" in the PR always actually means "height", or "rows", or "max_height". Like, "term_max_height" or "frame_height"/"frame_max_height".

Overall, I think the "max height" should always be detected automatically. If you want to expose the set_height (set_max_height?) method, you must test corner cases: What if I resize the terminal to the height of two rows?

[BjornTheProgrammer]

If you want to bring a libc dependency, do it in a cross-platform way: the code should compile on Windows.

Yup, it could be a target libc with something like

[target.'cfg(unix)'.dependencies]
libc = "0.2"

Other than that, I think the "size" in the PR always actually means "height", or "rows", or "max_height". Like, "term_max_height" or "frame_height"/"frame_max_height".

Totally agree here, I think the best name would be rows though. Since (at least in unix) that is how it is treated.

Overall, I think the "max height" should always be detected automatically. If you want to expose the set_height (set_max_height?) method, you must test corner cases: What if I resize the terminal to the height of two rows?

I'll do some more testing, but thanks to @furbyhaxx the smaller than 3 corner cases should be handled.

Overall, I think the "max height" should always be detected automatically. If you want to expose the set_height (set_max_height?) method, you must test corner cases: What if I resize the terminal to the height of two rows?

I believe this is already the case? The default function detects the number of rows, and sets the max height to that by default.

[furbyhaxx]

We should also verify that your -3 margin is always a sane default.

I needed to change it to 5 to get a select with filter completely fit the terminal. But could be because of the type of terminal I use currently.

[BjornTheProgrammer]

Yeah, the 3 was there since that is just for the "Select an Item", the search, and the final empty closing line. Probably should at the very least include one or two items.

Other than that I just made a commit, which compiles for unix and windows, which should theoretically fix the ssh issue. Mind giving it a try? Anyone who has a windows computer should probably test the new commit as well.

[fadeevab]

Hi guys, a busy week, I remember about the PR, I'll try to get back to it "ASAP"

[fadeevab]

@BjornTheProgrammer We're gonna simplify and minimize your patch! :) See review comments. Thank you for your contribution!

[fadeevab]

This trick is not gonna work if you set filter_mode before setting the "max rows". Usually, these setters should be more or less pure/idempotent.

You should remove it here and check the "filter_mode" somewhere, perhaps, in the on callback incrementing or decrementing "1" as needed.

[fadeevab]

Let's just remove the window_ prefix :)

[fadeevab]

The better naming would be Frame. The module is managing a fixed-size "frame" within which content is displayed and scrolled, similar to a window frame.

[fadeevab]

pos -> offset

[fadeevab]

After renaming it's gonna be like

.skip(self.frame.get_offset())
.take(self.frame.get_max_rows())

[fadeevab]

I realized that it just not needed. Try to shrink your terminal to a few rows of height, and then run the cargo run --example filter: the list is gonna be forever of a few rows, even if you expand the terminal height.

So, with max_rows = usize::MAX by default, the lists are gonna be working in an old way, which is good for backward compatibility.

[fadeevab]

Closed in favor of PR #91

[fadeevab]

Selection lists now have a scrolling behavior after setting a max_rows number. Thanks @BjornTheProgrammer for providing this PR, which is mainly used for the implementation, I just polished it and refactored.