-
Notifications
You must be signed in to change notification settings - Fork 284
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
add language bar #585
add language bar #585
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 awesome!
Minor nitpicks, but overall LGTM 👍
@@ -304,27 +304,58 @@ impl Info { | |||
} | |||
|
|||
fn get_language_field(&self, title: &str) -> String { | |||
let mut language_field = String::from(""); | |||
|
|||
let language_bar_length = 26; |
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.
Based on the screenshot, this seems to line up nicely. If this was 29 characters, it might be more symmetrical with the ASCII art, which maxes out at 40 characters wide. (29 + "Languages: ".len()
).
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.
If the language bar's length were to be fixed, this would indeed make sense. However with the possible extra characters, this value can go up to 35, or 46 with "Languages: ".len()
.
Collects the language bar to a string to reduce String mutations.
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.
Thanks for addressing suggestions!
#584
Remarks:
The language bar is 26 characters long (arbitrary value). However, in some cases it can be longer. Indeed, if a language's weight in the distribution is too small to account for a whole character, I do a cmp::Max(val, 1) so that, it will still appear in the language bar. This may not be optimal 🤔
As discussed before, if the terminal doesn't support true color, we fallback to a default color palette: