fix: case-insensitive language comparison #204
Conversation
Made a fix to make language comparisons case-insenstitive for excluded language Added few test cases
WalkthroughThe pull request introduces modifications to several files related to the handling of language exclusions in API responses. Key changes include enhanced validation for query parameters, improved error handling, and consistent formatting of excluded languages by converting them to lowercase. The updates affect both the logic for generating SVG cards and the underlying data processing functions, ensuring that exclusions are case-insensitive. Additionally, test cases have been updated and expanded to verify these changes, particularly focusing on the new case-insensitive exclusion logic. Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
api/cards/most-commit-language.ts (1)
24-25: Consider extracting common language exclusion logic.The language exclusion logic is duplicated across multiple files. Consider extracting this into a shared utility function to maintain consistency and reduce duplication.
Example implementation:
// src/utils/language-utils.ts export const processExcludedLanguage = (language: string): string => { const translatedLanguage = translateLanguage(language); return translatedLanguage.toLowerCase(); };Then use it in both files:
excludeArr.push(processExcludedLanguage(val));tests/github-api/commits-per-language.test.ts (1)
44-54: Consider consistent indentation in mock dataThe indentation of the new mock data entry appears to be slightly inconsistent with the surrounding code.
- { - repository: { - primaryLanguage: { - name: 'Jupyter Notebook', - color: '#f18e33' - } - }, - contributions: { - totalCount: 75 - } - }, + { + repository: { + primaryLanguage: { + name: 'Jupyter Notebook', + color: '#f18e33' + } + }, + contributions: { + totalCount: 75 + } + },tests/github-api/repos-per-language.test.ts (1)
138-138: Add space after comma in array for better readabilityMinor style improvement suggestion.
-const repoData = await getRepoLanguages('vn7n24fzkq', ['rust','jupyter notebook']); +const repoData = await getRepoLanguages('vn7n24fzkq', ['rust', 'jupyter notebook']);src/app.ts (1)
Line range hint
42-45: Consider applying case-insensitive handling to the action entry point.For consistency, consider applying the same case-insensitive handling to the exclude array in the GitHub Action entry point. Currently, only the CLI entry point handles case sensitivity.
const exclude = core.getInput('EXCLUDE', {required: false}).split(','); + const normalizedExclude = exclude.map(lang => + translateLanguage(lang).toLowerCase() + ); core.info(`Excluded languages: ${exclude}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
api/cards/most-commit-language.ts(1 hunks)api/cards/repos-per-language.ts(1 hunks)src/app.ts(2 hunks)src/github-api/commits-per-language.ts(1 hunks)src/github-api/repos-per-language.ts(1 hunks)tests/github-api/commits-per-language.test.ts(3 hunks)tests/github-api/repos-per-language.test.ts(2 hunks)
🔇 Additional comments (6)
api/cards/repos-per-language.ts (1)
24-25: LGTM! Verify empty string handling.
The implementation correctly implements case-insensitive language comparison by translating first and then converting to lowercase. This order is important as some translations might involve case-sensitive mappings.
Let's verify the handling of empty strings:
✅ Verification successful
Empty strings are handled safely
The implementation of translateLanguage safely handles empty strings by falling back to the default case which capitalizes the first character. For empty strings, this will simply return an empty string since charAt(0) and slice(1) on empty strings return empty strings.
The code is robust and handles all edge cases appropriately:
- If language exists in dictionary -> returns mapped value
- If language doesn't exist (including empty string) -> capitalizes first letter and returns
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check how translateLanguage handles empty strings
ast-grep --pattern 'translateLanguage($empty)' | rg -A 5 'translateLanguage'
Length of output: 76
Script:
#!/bin/bash
# Let's first find the translateLanguage function definition
ast-grep --pattern 'function translateLanguage($_) {
$$$
}'
# Also search for any direct usage of translateLanguage
rg "translateLanguage" -A 3 -B 3
Length of output: 69898
Script:
#!/bin/bash
# Let's find the translateLanguage function implementation
fd "translator.ts" --exec cat {} \;
Length of output: 7844
src/github-api/repos-per-language.ts (1)
88-88: LGTM! Verify consistency across similar functions.
The implementation correctly performs case-insensitive comparison using toLowerCase().
Let's verify the consistency of this approach across similar functions:
✅ Verification successful
Let me try a more focused search to find language comparison patterns in the codebase.
Let me try another search to find language exclusion patterns more effectively:
Case-insensitive language comparison is consistently implemented across the codebase
The search results confirm that case-insensitive language comparison using toLowerCase() is consistently implemented throughout the codebase:
- In API endpoints (
api/cards/), language names are converted to lowercase when building the exclude array - In GitHub API handlers (
src/github-api/), language comparisons usetoLowerCase()when checking against exclude list - In the main app (
src/app.ts), languages are also converted to lowercase when processing exclusions
The implementation in repos-per-language.ts follows this established pattern correctly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for similar language comparison patterns
# Look for other language comparison implementations
ast-grep --pattern 'includes($lang)' | rg -B 5 -A 5 'includes'
# Look for other toLowerCase usage with language comparisons
rg -B 5 -A 5 'toLowerCase.*language'
Length of output: 65638
Script:
#!/bin/bash
# Look for language name comparisons specifically
rg -B 3 -A 3 "\.toLowerCase.*lang|lang.*\.toLowerCase"
# Look for language exclusion patterns
rg -B 3 -A 3 "exclude.*lang|lang.*exclude"
Length of output: 66837
Script:
#!/bin/bash
# Search for language exclusion patterns in source files
rg -g '*.ts' -B 3 -A 3 'exclude.*language|language.*exclude'
# Search for toLowerCase usage with excludes
rg -g '*.ts' -B 3 -A 3 'exclude.*toLowerCase|toLowerCase.*exclude'
Length of output: 3072
src/github-api/commits-per-language.ts (1)
86-86: LGTM: Case-insensitive language comparison implemented correctly
The conversion of language name to lowercase before checking exclusions ensures consistent case-insensitive matching, which aligns with the PR objectives.
tests/github-api/commits-per-language.test.ts (1)
102-112: LGTM: Comprehensive test coverage for case-insensitive exclusion
The test case effectively validates the case-insensitive language comparison functionality by checking the exclusion of "jupyter notebook" against "Jupyter Notebook".
tests/github-api/repos-per-language.test.ts (1)
135-145: LGTM: Well-structured test for case-insensitive language exclusion
The test case effectively validates both case-insensitive comparison and proper handling of language names containing whitespace.
src/app.ts (1)
147-148: LGTM! Case-insensitive language comparison implemented correctly.
The changes effectively implement case-insensitive language comparison by converting translated languages to lowercase before adding them to the exclude array. This ensures consistent language matching regardless of the input case.
|
Awesome! |
Made a fix to make language comparisons case-insensitive for excluded language
Added few test cases
#203
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests