Skip to content

Conversation

alexs-sh
Copy link
Contributor

About
This change allows using the default terminal size if it cannot be detected. Information about the error is logged. The default value is 80 columns by 24 rows.

Issue
#14101

Before
image

After
image

Note

This is an initial version for reviewing and discussion. There are two points I want to discuss and improve:

  • Applying default values makes the Result usless at size()
  • It could lead to many duplicate messages in the log when size() is called repeatedly. It might make sense to save the information about using the default value and log the failure only once.

What do you think?

Use the default terminal size if the size cannot be detected. The
default value is 80 columns by 24 rows.
@the-mikedavis
Copy link
Member

Yeah let's eliminate all of the io::Results around the Rects starting in the Backend trait. It will be a larger change but then we don't have to unwrap these results all over the place.

I'm not too worried about logging multiple times, I believe we only call the size function a few times here and there.

@alexs-sh
Copy link
Contributor Author

@the-mikedavis Thx.

"let's eliminate all of the io::Results around the Rects starting in the Backend trait. " - just wanted to clarify a bit where to start removing the Result. As I understand, you’re suggesting removing Result even from the Backend trait.

For me, having an I/O Result in the Backend trait is fine. There are many ways the Backend can fail when reading the size. So, it’s very desirable to have a Result here to be able to signal any problems.

pub trait Backend {
...
    fn size(&self) -> Result<Rect, io::Error>;
...
}

Updating the signature in Terminal looks better to me. Here, we are applying the default values and converting the size, which could fail, into an infallible one.

impl<B> Terminal<B>
where
    B: Backend,
{
...
    pub fn size(&self) -> io::Result<Rect> {
...
}

PS: I still think it might be better to have a fallback constant. This way, we can preserve the signature (Backend / Terminal with Result look good and consistent) and explicitly use the default values using native Result functionality.
Something like:

    pub fn autoresize(&mut self) -> io::Result<Rect> {
        let size = self.size().unwrap_or(DEFAULT_TERMINAL_SIZE);
        if size != self.viewport.area {
            self.resize(size)?;
        };
        Ok(size)
    }

PPS:I can prepare several PRs so we can choose the best approach.

@the-mikedavis
Copy link
Member

Changing it just in Terminal is also alright, I don't have strong feelings about changing Backend. A constant for the default Rect also sounds like a good idea to me.

@alexs-sh
Copy link
Contributor Author

alexs-sh commented Sep 23, 2025

@the-mikedavis

#14486 - here is a PR that adds a constant for the default terminal size. For me, it does exactly what I need and eliminates the unwanted size → size calls.

#14487 - here is a PR also solves the problem by making size from Terminal infallible

So, we can review/compare and select the best approach.

@the-mikedavis
Copy link
Member

Closing in favor of #14487

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants