Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: simplify get_height_of_terminal() and get_width... #436

Merged
merged 1 commit into from
Sep 16, 2024

Conversation

muzimuzhi
Copy link
Contributor

@muzimuzhi muzimuzhi commented Aug 30, 2024

The eminence/terminal-size#41 PR mentioned in comments of get_(height|width)_of_terminal() (added by c36ca33)

dust/src/main.rs

Lines 83 to 91 in b636086

fn get_height_of_terminal() -> usize {
// Simplify once https://github.com/eminence/terminal-size/pull/41 is
// merged
terminal_size()
// Windows CI runners detect a terminal height of 0
.map(|(_, Height(h))| max(h as usize, DEFAULT_NUMBER_OF_LINES))
.unwrap_or(DEFAULT_NUMBER_OF_LINES)
- 10
}

has been merged and shipped since terminal-size v0.2.2. As we're using terminal-size v0.2.6 now, it's time to simplify them.

dust/Cargo.lock

Lines 766 to 768 in 38c4d23

[[package]]
name = "terminal_size"
version = "0.2.6"

Not sure if I made it to the most simplified form, as I can hardly speak Rust.

cc @Lucretiel

@Lucretiel
Copy link
Contributor

I think my original idea was that we'd just use the Height and Width types directly, rather than casting to usize. That was previously impossible in the absence of those traits.

@muzimuzhi
Copy link
Contributor Author

muzimuzhi commented Aug 30, 2024

Then that's out of my current Rust knowledge. Sorry, I'm closing this PR.

@muzimuzhi muzimuzhi closed this Aug 30, 2024
@bootandy
Copy link
Owner

bootandy commented Sep 3, 2024

I think its easier to convert to usize as I start doing math with it. eg: For height I use it to work out how many rows I can have displayed on the terminal.

I'd be happy to accept this pr as I think using .into() is slightly cleaner.

@muzimuzhi muzimuzhi reopened this Sep 4, 2024
@bootandy bootandy merged commit 86b2bd9 into bootandy:master Sep 16, 2024
17 checks passed
@muzimuzhi muzimuzhi deleted the simplify-get-terminal-wh branch September 16, 2024 21:06
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.

3 participants