Double pinyin query

[VictoriousRaptor]

This pull request introduces a comprehensive set of changes to enhance Pinyin support in the application, including the addition of Double Pinyin (“双拼”) functionality, improved translation mappings, and related settings. The most significant updates include the implementation of the IAlphabet interface, the refactoring of the PinyinAlphabet class, the introduction of Double Pinyin schemas, and updates to the user settings and localization files.

UI in General Panel

Pinyin and Double Pinyin Enhancements:

• Added the IAlphabet interface to define methods for language translation and determining translatability. (Flow.Launcher.Infrastructure/IAlphabet.cs)
• Refactored the PinyinAlphabet class to support Double Pinyin, utilize a cache for translations, and dynamically reload schemas based on user settings. (Flow.Launcher.Infrastructure/PinyinAlphabet.cs)
• Introduced Double Pinyin schemas as an enum and added settings for enabling/disabling Double Pinyin and selecting schemas. (Flow.Launcher.Infrastructure/UserSettings/Settings.cs) [1] [2]

Translation Mapping Improvements:

• Reimplemented the TranslationMapping class to simplify and optimize the mapping of original to translated indices. (Flow.Launcher.Infrastructure/TranslationMapping.cs)
• Added unit tests for TranslationMapping to validate its functionality. (Flow.Launcher.Test/TranslationMappingTest.cs)

Localization and Resource Updates:

• Updated localization files to include strings for Double Pinyin settings and schemas. (Flow.Launcher/Languages/en.xaml)
• Added double_pinyin.json to project resources for schema definitions. (Flow.Launcher/Flow.Launcher.csproj)

Codebase Simplifications:

• Updated methods in StringMatcher.cs to use the new ShouldTranslate method from IAlphabet. (Flow.Launcher.Infrastructure/StringMatcher.cs)
• Made helper methods IsAcronym and IsAcronymCount static for better encapsulation. (Flow.Launcher.Infrastructure/StringMatcher.cs) [1] [2]

[taooceros]

@VictoriousRaptor what's the status of this pr?

[VictoriousRaptor]

@VictoriousRaptor what's the status of this pr?

POC. I'm using it daily and so far so good. Since there're many double pinyin mappings we should make it user configurable. Is it acceptable to allow users to edit a json file that defines the mapping?

[taooceros]

well I can't think anything better...maybe I do want this to be plugin-able, but can't think of a good way to design the api.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

Flow.Launcher.Infrastructure/PinyinAlphabet.cs (1)

137-144: Consider adding input validation for robustness.

While the current implementation is functional, adding a simple validation check could prevent potential issues.

 private string ToDoublePin(string fullPinyin)
 {
+    if (string.IsNullOrEmpty(fullPinyin))
+        return fullPinyin;
+        
     if (currentDoublePinyinTable.TryGetValue(fullPinyin, out var doublePinyinValue))
     {
         return doublePinyinValue;
     }
     return fullPinyin;
 }

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d537ce2 and 1302021.

📒 Files selected for processing (5)

• Flow.Launcher.Infrastructure/PinyinAlphabet.cs (1 hunks)
• Flow.Launcher.Infrastructure/UserSettings/Settings.cs (5 hunks)
• Flow.Launcher/Languages/en.xaml (1 hunks)
• Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (3 hunks)
• Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• Flow.Launcher.Infrastructure/UserSettings/Settings.cs

🧰 Additional context used

🧠 Learnings (1)

Flow.Launcher/Languages/en.xaml (1)

Learnt from: taooceros
PR: Flow-Launcher/Flow.Launcher#2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.

🪛 GitHub Check: Check Spelling

Flow.Launcher/Languages/en.xaml

[warning] 114-114:
Neng is not a recognized word. (unrecognized-spelling)

---

[warning] 114-114:
Neng is not a recognized word. (unrecognized-spelling)

---

[warning] 113-113:
Ruan is not a recognized word. (unrecognized-spelling)

---

[warning] 113-113:
Ruan is not a recognized word. (unrecognized-spelling)

---

[warning] 111-111:
Xiao is not a recognized word. (unrecognized-spelling)

---

[warning] 111-111:
Xiao is not a recognized word. (unrecognized-spelling)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: build

🔇 Additional comments (9)

Flow.Launcher.Infrastructure/PinyinAlphabet.cs (3)

24-37: LGTM! Clean dependency injection and reactive pattern implementation.

The constructor properly initializes dependencies and sets up reactive updates for settings changes.

---

78-82: Good fix for the translation eligibility logic.

The method properly addresses the past review concern about rejecting valid double-pinyin queries with odd lengths. The logic is now more straightforward and user-friendly.

---

94-133: Excellent fixes addressing past review concerns.

The implementation now correctly:

• Updates the previousIsChinese flag (line 114)
• Maps indices to point to the actual translated text, not spaces
• Handles both Chinese and non-Chinese character transitions properly

The past review concerns about off-by-one errors and flag updates have been resolved.

Flow.Launcher/Languages/en.xaml (1)

108-120: Comprehensive localization support for double pinyin feature.

The localization strings properly support the new double pinyin functionality. The static analysis warnings about "Neng", "Ruan", and "Xiao" are false positives - these are legitimate Chinese words/names used in the pinyin schema names.

Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml (1)

350-387: Well-structured hierarchical UI for pinyin settings.

The grouped card layout with conditional visibility creates an intuitive user experience:

• Base pinyin setting controls overall feature
• Double pinyin toggle only appears when pinyin is enabled
• Schema selection only appears when double pinyin is enabled

The data binding and visibility logic is correctly implemented.

Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (4)

38-38: Good addition of dropdown data class for double pinyin schemas.

Consistent with the existing pattern used for other dropdown data classes in the view model.

---

267-275: Excellent dependency management between pinyin settings.

The logic properly ensures that UseDoublePinyin is automatically disabled when ShouldUsePinyin is set to false, maintaining consistent state and preventing invalid configurations.

---

277-284: Clean implementation of double pinyin properties.

The UseDoublePinyin property and DoublePinyinSchemas list follow the established patterns in the view model and properly support the UI binding requirements.

---

181-181: Proper localization update for the new dropdown.

Ensures the double pinyin schema dropdown labels are updated when language changes, maintaining consistency with other dropdown controls.

[Copilot]

Pull Request Overview

Adds support for Double Pinyin in search by exposing new toggles in the UI, loading a JSON-based mapping table, and wiring it through settings, view models, and the PinyinAlphabet.

• UI: new switch and combo box for enabling double-pinyin and selecting a schema
• Settings & ViewModel: new UseDoublePinyin and DoublePinyinSchema properties, with binding and localization
• Core logic: JSON file of mappings, loading in PinyinAlphabet, and a new TranslationMapping class with unit tests

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
SettingPages/Views/SettingsPaneGeneral.xaml	Added cards/toggles and schema selector
SettingsPaneGeneralViewModel.cs	Introduced UseDoublePinyin, schema list
Resources/double_pinyin.json	New mapping table for all supported schemas
Languages/en.xaml	Added localization for toggles and schemas
Flow.Launcher.csproj	Ensured JSON is copied to output
TranslationMapping.cs	New mapping utility with AddNewIndex/MapToOriginalIndex
TranslationMappingTest.cs	Unit tests for TranslationMapping
UserSettings/Settings.cs	Added settings, JSON serialization, enum
Infrastructure/StringMatcher.cs	Switched to ShouldTranslate predicate
Infrastructure/PinyinAlphabet.cs	Loads JSON, applies double-pinyin translation
Infrastructure/IAlphabet.cs	Updated interface to use ShouldTranslate

Comments suppressed due to low confidence (3)

Flow.Launcher.Infrastructure/TranslationMapping.cs:29

• Method name 'endConstruct' doesn't follow .NET naming conventions; consider renaming to 'EndConstruct'.

        public void endConstruct()

Flow.Launcher.Infrastructure/TranslationMapping.cs:29

• The behavior of endConstruct isn't covered by unit tests; consider adding tests to verify it prevents further calls to AddNewIndex after construction.

        public void endConstruct()

Flow.Launcher/Flow.Launcher.csproj:130

• Using 'Update' for a new resource entry may not include the JSON in the build; consider using 'Include' or verify that default globbing picks it up so it’s reliably copied under the Resources folder at runtime.

    <None Update="Resources\double_pinyin.json">

[Jack251970]

LGTM

Reviewed by Jack251970.