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

Fix 'attempt to divide by zero' panic #6155

Merged
merged 1 commit into from
Mar 3, 2023
Merged

Fix 'attempt to divide by zero' panic #6155

merged 1 commit into from
Mar 3, 2023

Conversation

nuid64
Copy link
Contributor

@nuid64 nuid64 commented Mar 2, 2023

Helix crashed when I cut the window too much.

thread 'main' panicked at 'attempt to divide by zero', helix-term/src/ui/text.rs:61:23
stack backtrace:
   0: rust_begin_unwind
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/core/src/panicking.rs:64:14
   2: core::panicking::panic
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/core/src/panicking.rs:111:5
   3: helix_term::ui::text::required_size
   4: <helix_term::ui::markdown::Markdown as helix_term::compositor::Component>::required_size
   5: <helix_term::ui::popup::Popup<T> as helix_term::compositor::Component>::required_size
   6: <helix_term::ui::popup::Popup<T> as helix_term::compositor::Component>::render
   7: helix_term::application::Application::render::{{closure}}
   8: helix_term::application::Application::run::{{closure}}
   9: tokio::runtime::park::CachedParkThread::block_on
  10: tokio::runtime::runtime::Runtime::block_on
  11: hx::main

@@ -58,7 +58,7 @@ pub fn required_size(text: &tui::text::Text, max_text_width: u16) -> (u16, u16)
let content_width = content.width() as u16;
if content_width > max_text_width {
text_width = max_text_width;
height += content_width / max_text_width;
height += content_width.checked_div(max_text_width).unwrap_or(0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You want to use saturating_div instead which avoids the unwrap

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I guess that doesn't check for 0

@nuid64
Copy link
Contributor Author

nuid64 commented Mar 2, 2023

Also I found two required_size functions in helix-term/src/ui/text.rs and one of them returns (u16, u16) not wrapped in Option. It's confusing to have two of them, both of which perform actions on the text.

@archseer archseer merged commit ddc5bf4 into helix-editor:master Mar 3, 2023
@nuid64 nuid64 deleted the fix_div_by_zero branch March 4, 2023 09:02
estin pushed a commit to estin/helix that referenced this pull request Mar 4, 2023
wes-adams pushed a commit to wes-adams/helix that referenced this pull request Jul 4, 2023
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.

None yet

2 participants