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 support for displaying most used language (or 2nd, 3rd, 4th, ...) #209

Merged
merged 31 commits into from
Jul 9, 2023

Conversation

elliotwutingfeng
Copy link
Contributor

@elliotwutingfeng elliotwutingfeng commented May 3, 2023

Closes #87

Continues where #96 left off. Thank you @spenserblack for starting this.

Display the n-th most used language by enabling ?showLanguage=true and using the ?languageRank=n query string, where n ∈[1, number of languages detected by tokei]

For example,

https://tokei-rs.onrender.com/b1/github/XAMPPRocky/tokei?showLanguage=true&languageRank=1&label=Champion&style=for-the-badge)](https://github.com/XAMPPRocky/tokei

Edge cases

  • If ?showLanguage=true is not present, line count will be displayed as per default badge behavior.
  • If ?showLanguage=true but ?languageRank= not present, badge will display most used language.
  • If n ∉[1, number of languages detected by tokei], badge will display "N/A".
  • If no languages are detected by tokei, badge will display "No Language".

Live Demo

Repo: https://github.com/XAMPPRocky/tokei

Champion

First Runner Up

Third Place

languageRank0

languageRank 100

languageRank 10000000000000000000000000000000000000000

No Languages

Colors, badge styles, logos, and labels can be freely customized by the user.


It is possible to enable negative integers and get least-used, 2nd least used languages, ..., but this would probably be out of scope here.

Is ranking a suitable query parameter name?

@elliotwutingfeng elliotwutingfeng marked this pull request as ready for review May 3, 2023 07:51
@spenserblack
Copy link

spenserblack commented May 3, 2023

Thanks for doing this! Reusing the same endpoint and adding some more query parameters makes the code look much more elegant than what I was doing 😆

Is ranking a suitable query parameter name?

Just an idea for slightly more clear usage:
The endpoint showing lines-of-code by default, and the name of a language with ?ranking=1 might be a bit confusing IMO. A boolean parameter like ?getLanguage might be more clear. You could be strict and check if the paremeter == "true", or perhaps you can just check if the parameter exists in the query parameters. When this is used, by default ranking=1. This could potentially make the ranking parameter a bit more clear -- it's the ranking for getLanguage. And this way ranking has a singular purpose, to get the nth language, and doesn't have the additional purpose of being a flag to display the language name.

Values passed to ?ranking= will be capped at number of languages detected by tokei. For example, if tokei detects 6 languages in a repo, and ?ranking=300 is specified, 300 will be capped to 6.

This might make sense where the number of languages is known, but might be a bit awkward when the number of languages is unknown and/or frequently change. Might make more sense for out-of-bounds language rankings to simply be N/A. For example, if I wanted to display the top n languages of a project, and the actual number of languages went from >= n to < n, it might be inconvenient if the badges started to look like this: 1st: Lang1, 2nd: Lang2, ..., (n-1)th: LastLang, nth: LastLang. This would require action on my part to delete one or more badges to make the results make more sense, while N/A for out-of-bounds rankings could be left alone and still look "clean."

@elliotwutingfeng elliotwutingfeng marked this pull request as draft May 4, 2023 16:30
@elliotwutingfeng elliotwutingfeng marked this pull request as ready for review May 5, 2023 15:17
@XAMPPRocky
Copy link
Owner

Thank you for your PR! Sorry for the delay, but now that I merged your other PR, this has conflicts. Would you be interested in updating this one?

Earthfile Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
@elliotwutingfeng elliotwutingfeng marked this pull request as draft July 8, 2023 13:17
@elliotwutingfeng
Copy link
Contributor Author

Haven't looked into this for a while, will try deploying on onrender.com

@elliotwutingfeng elliotwutingfeng marked this pull request as ready for review July 9, 2023 15:23
@XAMPPRocky
Copy link
Owner

Thank you for your PR!

@XAMPPRocky XAMPPRocky merged commit 8ccebaa into XAMPPRocky:master Jul 9, 2023
@elliotwutingfeng elliotwutingfeng deleted the ranking branch July 9, 2023 17:03
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.

Primary language badge
3 participants