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

add language bar #585

Merged
merged 6 commits into from
Feb 20, 2022
Merged

add language bar #585

merged 6 commits into from
Feb 20, 2022

Conversation

o2sh
Copy link
Owner

@o2sh o2sh commented Feb 19, 2022

#584

Screenshot 2022-02-19 at 23 21 45

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:

Screenshot 2022-02-19 at 23 49 27

@o2sh o2sh changed the title add language bar (#584) add language bar Feb 19, 2022
Copy link
Collaborator

@spenserblack spenserblack left a 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;
Copy link
Collaborator

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()).

Copy link
Owner Author

@o2sh o2sh Feb 20, 2022

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().

src/info/mod.rs Outdated Show resolved Hide resolved
src/info/mod.rs Outdated Show resolved Hide resolved
src/info/mod.rs Outdated Show resolved Hide resolved
o2sh and others added 2 commits February 20, 2022 12:31
Collects the language bar to a string to reduce String mutations.
Copy link
Collaborator

@spenserblack spenserblack left a 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!

@o2sh o2sh merged commit e9e20bb into main Feb 20, 2022
@o2sh o2sh deleted the feat/add-language-bar branch February 20, 2022 18:54
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