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

Render the help message as the default no-content status message. #222

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

NickGeek
Copy link

Fixes #221. Also keeps the help message visible after resizing the window.

@codecov
Copy link

codecov bot commented Mar 15, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.05 ⚠️

Comparison is base (c2640f2) 42.51% compared to head (a6dc526) 42.46%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #222      +/-   ##
==========================================
- Coverage   42.51%   42.46%   -0.05%     
==========================================
  Files          10       10              
  Lines         915      916       +1     
==========================================
  Hits          389      389              
- Misses        526      527       +1     
Flag Coverage Δ
x86_64-apple-darwin 42.46% <0.00%> (-0.05%) ⬇️
x86_64-unknown-linux-gnu 42.35% <0.00%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/editor.rs 28.29% <0.00%> (-0.12%) ⬇️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@NickGeek
Copy link
Author

Ah damn, went over the line limit thanks to clippy. I'll look into that tomorrow.

@NickGeek
Copy link
Author

NickGeek commented Mar 16, 2023

Hmm, I've found a bug when the status bar line goes >1 row in size the main text buffer's view breaks a bit.

Edit: fixed!

@ilai-deutel
Copy link
Owner

Sorry for the delay, I'll review this very soon!

@all-contributors please add @ NickGeek for code

Repository owner deleted a comment from allcontributors bot Mar 21, 2023
@ilai-deutel
Copy link
Owner

@all-contributors please add @NickGeek for code

@allcontributors
Copy link
Contributor

@ilai-deutel

I've put up a pull request to add @NickGeek! 🎉

Copy link
Owner

@ilai-deutel ilai-deutel left a comment

Choose a reason for hiding this comment

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

Thank you, this is a great change!

src/editor.rs Outdated
@@ -291,7 +289,8 @@ impl Editor {
fn update_window_size(&mut self) -> Result<(), Error> {
let wsize = sys::get_window_size().or_else(|_| terminal::get_window_size_using_cursor())?;
// Make room for the status bar and status message
(self.screen_rows, self.window_width) = (wsize.0.saturating_sub(2), wsize.1);
(self.screen_rows, self.window_width) =
(wsize.0.saturating_sub(1 + (self.status_msg().len() + wsize.1) / wsize.1), wsize.1);
Copy link
Owner

Choose a reason for hiding this comment

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

Should it be

Suggested change
(wsize.0.saturating_sub(1 + (self.status_msg().len() + wsize.1) / wsize.1), wsize.1);
(wsize.0.saturating_sub(1 + (self.status_msg().len() + wsize.1 - 1) / wsize.1), wsize.1);

instead? Currently, 1 + (self.status_msg().len() + wsize.1) / wsize.1 is equivalent to 2 + self.status_msg().len() / wsize.1

Copy link
Author

Choose a reason for hiding this comment

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

Hmm, not quite sure how to fit this one one line with this change + keeping rustfmt happy.

Copy link
Owner

Choose a reason for hiding this comment

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

I think you need at least one extra line

-        (self.screen_rows, self.window_width) =
-            (wsize.0.saturating_sub(1 + (self.status_msg().len() + wsize.1 - 1) / wsize.1), wsize.1);
+        self.screen_rows =
+            wsize.0.saturating_sub(1 + (self.status_msg().len() + wsize.1 - 1) / wsize.1);
+        self.window_width = wsize.1;

At least until div_ceil is stabilized :)

I just merged #229, which cuts down the total line count, so that should be fine now

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.

Keep shortcut help visible
2 participants