Introduce Ew2uEnabled flag for w to ư transformation#4
Introduce Ew2uEnabled flag for w to ư transformation#4nhktmdzhg merged 1 commit intoLotusInputMethod:masterfrom
Conversation
…update Telex input method definitions.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the Telex input method by introducing a configurable feature flag, Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a feature flag Ew2uEnabled to make the w to ư transformation optional in the Telex input method. The implementation correctly adds the flag and updates the transformation logic. The input method definitions are also simplified by removing the now-redundant 'Telex W' variant. The changes are well-tested. I've left a couple of suggestions to improve code clarity and remove some redundant logic.
| if transformations[0].Rule.Result == 'w' { | ||
| transformations[0].Rule.Result = 'ư' | ||
| transformations[0].Rule.EffectOn = 'ư' | ||
| } else if transformations[0].Rule.Result == 'W' { | ||
| transformations[0].Rule.Result = 'Ư' | ||
| transformations[0].Rule.EffectOn = 'Ư' | ||
| } |
There was a problem hiding this comment.
The else if condition checking for transformations[0].Rule.Result == 'W' will never be met. The Rule.Result for an appending transformation is always the lowercase version of the character. The casing is handled by the IsUpperCase flag on the Transformation struct, which is later processed by the Flatten function. This else if block is dead code and should be removed for clarity and to prevent future confusion.
if transformations[0].Rule.Result == 'w' {
transformations[0].Rule.Result = 'ư'
transformations[0].Rule.EffectOn = 'ư'
}| } | ||
|
|
||
| // Flag ON - Start of word | ||
| ng = NewEngine(im, EstdFlags|Ew2uEnabled) |
There was a problem hiding this comment.
The expression EstdFlags|Ew2uEnabled is redundant. Since Ew2uEnabled has been added to the definition of EstdFlags, EstdFlags already includes this flag by default. Using just EstdFlags would be clearer and less prone to confusion.
| ng = NewEngine(im, EstdFlags|Ew2uEnabled) | |
| ng = NewEngine(im, EstdFlags) |
This pull request introduces a new feature flag to control the mapping of the
wkey to the Vietnamese characterưin the Telex input method, along with related changes to input method definitions and tests. The most important changes are grouped below:Feature flag addition and logic updates
Ew2uEnabledto the engine flags, and updatedEstdFlagsto include this flag. This allows toggling the behavior of mappingwtoư. (bamboo.go)BambooEngineto convertworWtoưorƯwhen theEw2uEnabledflag is set. (bamboo.go)Input method definition changes
wto remove the explicit mapping toƯwhen typed alone; now,wonly maps toƯƠĂ. (input_method_def.go)input_method_def.go)Testing updates
Ew2uEnabledflag, verifying correct behavior when the flag is on or off, and for different positions and cases ofw. (bamboo_test.go)]) forưinstead ofw, reflecting the updated mapping. (bamboo_test.go)