Add System Language Item #3161
Add System Language Item #3161
Conversation
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughThis pull request introduces enhanced language handling capabilities in the Flow Launcher Core. The changes focus on improving internationalization support by adding a new method to retrieve system translations and modifying the language change mechanism. The updates include a new Changes
Possibly related PRs
Suggested labels
Poem
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
🧹 Nitpick comments (2)
Flow.Launcher.Core/Resource/Internationalization.cs (2)
181-184: Inserting system language at the top of the list
Prepending the system language inLoadAvailableLanguagesensures immediate visibility in the UI. As a possible improvement, consider whether sorting or grouping logic should be flexible, e.g., if other future “special” languages need to appear.
186-209: Implementing a straightforward system language resolution
By iterating through the available languages, this code finds a match or defaults to English. This is fine for a moderate list of languages. If performance or loading time becomes a concern in the future, a mapping/dictionary approach may be more optimal.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Flow.Launcher.Core/Resource/AvailableLanguages.cs(1 hunks)Flow.Launcher.Core/Resource/Internationalization.cs(5 hunks)
🔇 Additional comments (6)
Flow.Launcher.Core/Resource/Internationalization.cs (5)
21-22: Use of separate constants for system and default language is clear
DefiningSystemLanguageCodeandDefaultLanguageCodeas constants clarifies the distinction between system language handling and the fallback language.
73-81: Good logic for detecting and handling the system language
This section correctly checks if the user-suppliedlanguageCodeis the specialSystemLanguageCodeand then resolves the actual system-based code. Ensure consistent usage throughout the codebase, particularly in places wherelanguageCodemight be compared with "System".
82-84: Calls the new internal overload appropriately
Directly mapping the resolved language code to yourChangeLanguage(Language, bool)overload maintains clarity. Good job.
102-102: Changing method signature to private might break external references
Previously,ChangeLanguage(Language language)looked publicly exposed. Confirm there is no external usage of the old signature. If it was only used internally, making the method private is fine. Otherwise, consider backward compatibility.
117-117: Settings now explicitly captures system language
Using a ternary operator here to store either"System"or the actual language code helps you persist the user’s intent. This ensures the system language remains recognized upon future application restarts.Flow.Launcher.Core/Resource/AvailableLanguages.cs (1)
66-97: Method covering a broad set of languages for “System” translations
GetSystemTranslationswitches across all supported languages, returning a localized string for “System.” This approach is straightforward. For scalability, you might eventually consider a resource-based solution or something that can be updated without code changes.
This comment has been minimized.
This comment has been minimized.
|
🥷 Code experts: Yusyuriv Yusyuriv has most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
Flow.Launcher.Infrastructure/Constant.cs (1)
56-56: Consider verifying or clarifying naming consistency.
While using"System"as the label may be acceptable, you might want to explicitly mark it as a system-reserved or system-based language code in comments or docstring. This ensures clarity in cases where "System" might clash with other usages in the codebase or third-party library references.Flow.Launcher.Core/Resource/Internationalization.cs (2)
72-83: Consider adding error logging for system language resolution.The system language handling logic is well-structured. However, consider adding error logging when the system language code resolution fails, to help with troubleshooting.
if (languageCode == Constant.SystemLanguageCode) { languageCode = GetSystemLanguageCode(); + Log.Debug($"|Internationalization.ChangeLanguage|Resolved system language to: {languageCode}"); isSystem = true; }
185-208: Consider caching the system language code.The implementation thoroughly checks different culture codes, but since the system language doesn't change frequently, consider caching the result to avoid repeated culture info lookups and string comparisons.
private string GetSystemLanguageCode() { + // Cache the result as it rarely changes + if (_cachedSystemLanguageCode != null) + return _cachedSystemLanguageCode; + var availableLanguages = AvailableLanguages.GetAvailableLanguages(); // Retrieve the language identifiers for the current culture var currentCulture = CultureInfo.CurrentCulture; var twoLetterCode = currentCulture.TwoLetterISOLanguageName; var threeLetterCode = currentCulture.ThreeLetterISOLanguageName; var fullName = currentCulture.Name; // Try to find a match in the available languages list foreach (var language in availableLanguages) { var languageCode = language.LanguageCode; if (string.Equals(languageCode, twoLetterCode, StringComparison.OrdinalIgnoreCase) || string.Equals(languageCode, threeLetterCode, StringComparison.OrdinalIgnoreCase) || string.Equals(languageCode, fullName, StringComparison.OrdinalIgnoreCase)) { - return languageCode; + _cachedSystemLanguageCode = languageCode; + return _cachedSystemLanguageCode; } } - return DefaultLanguageCode; + _cachedSystemLanguageCode = DefaultLanguageCode; + return _cachedSystemLanguageCode; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Flow.Launcher.Core/Resource/Internationalization.cs(5 hunks)Flow.Launcher.Infrastructure/Constant.cs(1 hunks)Flow.Launcher.Infrastructure/UserSettings/Settings.cs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
🔇 Additional comments (4)
Flow.Launcher.Core/Resource/Internationalization.cs (4)
21-21: LGTM! Good use of a constant for the default language.The constant provides a clear fallback value and is used consistently throughout the code.
Line range hint
101-116: LGTM! Well-implemented language change handling.The implementation properly handles both system and specific language selections, with appropriate culture settings and async plugin updates.
180-183: LGTM! Clean implementation of system language option.The system language option is properly added to the top of the list with localized display text.
Line range hint
72-208: Verify system language switching behavior.The implementation looks solid, but please ensure comprehensive testing of the following scenarios:
- Initial selection of system language
- Switching between system and specific languages
- Application restart with system language selected
- Handling of unsupported system languages
Run the following script to check for potential edge cases in language handling:
✅ Verification successful
Implementation verified - system language switching is robust and safe.
The code handles all critical scenarios:
- Proper system language detection with multiple fallback mechanisms
- Consistent culture propagation across the application
- Safe handling of unsupported system languages
- Correct plugin notification of language changes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potential issues in language handling # Check for direct references to language codes that might bypass the system language logic rg -g '!*.{Designer.cs,resx,xaml}' -l '"en"|"zh"|"de"' --type cs # Check for consistency in language-related constant usage rg -g '!*.{Designer.cs,resx,xaml}' "SystemLanguageCode|DefaultLanguageCode" --type cs # Look for potential culture-related issues ast-grep --pattern 'CultureInfo.CurrentCulture' --lang csLength of output: 3435
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher.Core/Resource/Internationalization.cs(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
🔇 Additional comments (4)
Flow.Launcher.Core/Resource/Internationalization.cs (4)
21-21: LGTM! Good constant extraction.The addition of
DefaultLanguageCodereduces magic string duplication andSystemLanguageCodefield properly encapsulates the system language state.Also applies to: 26-26
31-31: LGTM! Proper initialization of readonly field.The
SystemLanguageCodeis correctly initialized in the constructor.
206-208: LGTM! Clean implementation of system language option.The system language option is properly added as the first item with appropriate translation.
98-109: LGTM! Well-structured language change handling.The implementation correctly:
- Handles system language detection
- Preserves the "system" setting while using actual language for UI
- Updates both culture and UI culture appropriately
Let's verify the language change behavior:
Also applies to: 127-142
✅ Verification successful
Verified: Language change implementation is properly centralized and consumed
The codebase correctly:
- Centralizes all culture changes in Internationalization.cs
- Uses CurrentCulture appropriately for formatting in other locations
- Maintains proper system language support without conflicts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify language change implementation # Look for other places in the codebase that might need similar system language handling # Check for direct CurrentCulture usage that might need updating rg "CultureInfo\.CurrentCulture" -l # Check for other language-related settings that might need system language support rg "Language\s*=\s*[\"']system[\"']" -lLength of output: 415
Script:
#!/bin/bash # Check how CurrentCulture is used in the identified files rg "CultureInfo\.CurrentCulture" -B 2 -A 2 # Also check for any Thread.CurrentThread.CurrentCulture usage rg "Thread.*CurrentCulture" -B 2 -A 2 # Look for any other culture-related settings rg "Culture(Info)?\." -B 2 -A 2Length of output: 15927
This comment has been minimized.
This comment has been minimized.
|
I just renamed the GetSystemLanguageCode to be more descriptive about its intended usage. |
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. Pattern suggestions ✂️ (1)You could add these patterns to If the flagged items are 🤯 false positivesIf items relate to a ...
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Flow.Launcher.Core/Resource/Internationalization.cs (1)
40-66: LGTM! Consider adding debug logging.The implementation correctly handles system language detection at startup with proper fallback. Consider adding debug logging when falling back to the default language to help troubleshoot language detection issues.
if (string.Equals(languageCode, twoLetterCode, StringComparison.OrdinalIgnoreCase) || string.Equals(languageCode, threeLetterCode, StringComparison.OrdinalIgnoreCase) || string.Equals(languageCode, fullName, StringComparison.OrdinalIgnoreCase)) { return languageCode; } } +Log.Debug($"|Internationalization.GetSystemLanguageCodeAtStartup|No matching language found for system culture {fullName}, falling back to {DefaultLanguageCode}"); return DefaultLanguageCode;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher.Core/Resource/Internationalization.cs(5 hunks)
🔇 Additional comments (5)
Flow.Launcher.Core/Resource/Internationalization.cs (5)
21-21: LGTM! Good constant extraction.Moving the default language code to a constant and making SystemLanguageCode readonly improves code maintainability.
Also applies to: 26-26
31-31: LGTM! Correctly initializes system language at startup.This initialization addresses the previous issue where CultureInfo.CurrentCulture could be affected by language changes.
99-111: LGTM! Clean system language handling.The changes correctly handle system language detection and delegation to the new overload.
208-210: LGTM! Clean system language option implementation.The system language option is correctly added at the start of the list with proper translation.
Line range hint
129-144: Consider potential race condition in async plugin update.The async plugin metadata update could potentially race with subsequent language changes. Consider using a cancellation token or synchronization mechanism.
Add support of system language item to help FL automatically select the system language as display language