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

Explain EXCLUDED_LANGS casing or make it case-insensitive #64

Closed
erikw opened this issue Feb 15, 2022 · 1 comment · Fixed by #65
Closed

Explain EXCLUDED_LANGS casing or make it case-insensitive #64

erikw opened this issue Feb 15, 2022 · 1 comment · Fixed by #65

Comments

@erikw
Copy link
Contributor

erikw commented Feb 15, 2022

The current implementation expects the languages to be in the same style as displayed on GitHub e.g.

  • HTML, not html
  • Ruby, not ruby

My first instict when I set up EXCLUDED_LANGS was to make something like html,tex. Not until reading the source code I saw that the code expects the exclude list to be using the same casing as GitHub has. Thus to exclude HTML and Tex files, we must write HTML,TeX.

I propose one of these:

  • We make the lookup case-insensitive i.e. normalize both each repo's languages and the user's set EXCLUDED_LANGS to lower-case.
    languages = await self.languages
    if name in self._exclude_langs:
  • If not, we should explain in the README that the EXCLUDED_LANGS should have languages as displayed in the generated image / as shown on the GitHub repo page
@jstrieb
Copy link
Owner

jstrieb commented Feb 15, 2022

Hey @erikw, thanks for the suggestion! I wholeheartedly agree that it should be a case-insensitive check. This is a good catch.

Feel free to make a PR, otherwise I can push this change at some point.

erikw added a commit to erikw/github-stats that referenced this issue Feb 16, 2022
It's not clear for users how the EXCLUDED_LANGS  should be populated:
* html,tex
* HTML,TeX
?

Avoid all question marks by normalizing both strings to
lower case before they are compared.

Fixes jstrieb/github-stats#64
erikw added a commit to erikw/github-stats-fork that referenced this issue Feb 16, 2022
It's not clear for users how the EXCLUDED_LANGS  should be populated:
* html,tex
* HTML,TeX
?

Avoid all question marks by normalizing both strings to
lower case before they are compared.

Fixes jstrieb#64
erikw added a commit to erikw/github-stats that referenced this issue Feb 16, 2022
It's not clear for users how the EXCLUDED_LANGS  should be populated:
* html,tex
* HTML,TeX
?

Avoid all question marks by normalizing both strings to
lower case before they are compared.

Fixes jstrieb/github-stats#64
erikw added a commit to erikw/github-stats-fork that referenced this issue Feb 16, 2022
It's not clear for users how the EXCLUDED_LANGS should be populated:
* html,tex
* HTML,TeX
?

Avoid all question marks by normalizing both strings to
lower case before they are compared.

Fixes jstrieb#64
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 a pull request may close this issue.

2 participants