feat: implement 3-state W2U toggle#5
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a modular W2U (w to ư) logic by adding a w2uMode field and a SetW2UMode method to the BambooEngine, replacing the previous flag-based implementation. Feedback includes a critical note regarding a regression where the new mode is not initialized from existing flags in NewEngine, which would disable the feature for current users. Additionally, it is recommended to simplify the boolean logic for canApplyW2U, replace magic numbers with named constants for better maintainability, and add input validation to the SetW2UMode method.
| canApplyW2U := false | ||
| if e.w2uMode == 1 { | ||
| if len(composition) > 0 { | ||
| canApplyW2U = true | ||
| } | ||
| } else if e.w2uMode == 2 { | ||
| canApplyW2U = true | ||
| } |
There was a problem hiding this comment.
The logic for determining canApplyW2U can be simplified for better readability.
Important: There is a potential regression here. The previous implementation checked the Ew2uEnabled flag. Since w2uMode defaults to 0 and is not initialized from the flags in NewEngine, existing users who rely on Ew2uEnabled (which is part of EstdFlags) will find the W2U feature disabled by default. Please ensure w2uMode is correctly initialized in NewEngine based on the provided flags.
| canApplyW2U := false | |
| if e.w2uMode == 1 { | |
| if len(composition) > 0 { | |
| canApplyW2U = true | |
| } | |
| } else if e.w2uMode == 2 { | |
| canApplyW2U = true | |
| } | |
| canApplyW2U := (e.w2uMode == 2) || (e.w2uMode == 1 && len(composition) > 0) |
| composition []*Transformation | ||
| inputMethod InputMethod | ||
| flags uint | ||
| w2uMode int // 0: Disabled, 1: Middle-Only, 2: Everywhere |
Implement a unified 3-state W2U toggle (Disabled, Middle-Only, Everywhere). Middle-only mode ignores 'w' at the start of words to prevent interference with English characters while providing Vietnamese shortcuts elsewhere.