-
-
Notifications
You must be signed in to change notification settings - Fork 52
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
Support configuring user gutter width #223
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good! I first chose 30 as the width before adding support for display names, so it's definitely a little wide with the other configurations.
There are a couple clippy warnings. I've commented on them and ways that I think you can improve on what it's recommending.
src/config.rs
Outdated
@@ -549,6 +552,7 @@ impl Tunables { | |||
open_command: self.open_command, | |||
notifications: self.notifications.unwrap_or_default(), | |||
image_preview: self.image_preview.map(ImagePreview::values), | |||
user_gutter_width: self.user_gutter_width.unwrap_or(30 as usize), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cargo clippy
suggests 30_usize
here, but I'm pretty sure you should just be able to do 30
and inference will figure it out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had problems with 30
getting inferred as u16
but I don't have that problem anymore. Maybe the LSP that was not working properly. Works as you suggested for me now.
Co-authored-by: Ulyssa <[email protected]>
309e7c5
to
a0d141a
Compare
Thank you for the quick review and suggestions. Hopefully everything looks good now. |
I found the user gutter too wide by default, especially when using localpart or display name for usernames so i made it configurable. Seems to work fine but I'm not a rustacean so there is perhaps a more idiomatic way to do this.