From 21adf971a0ca5fc5154209ce7e9271b8abe7e9d6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E5=B1=85=E6=88=8E=E6=B0=8F?= <chen.sst@gmail.com>
Date: Sun, 5 Feb 2023 20:19:05 +0800
Subject: [PATCH] fix(selector): do not match fallback modifier

Tab = Shift+Right should be skipped instead of treated as Right.

Fixes #609
---
 src/rime/gear/editor.cc                    |  3 +-
 src/rime/gear/key_binding_processor.h      | 10 ++++++-
 src/rime/gear/key_binding_processor_impl.h | 35 +++++++++++++---------
 src/rime/gear/navigator.cc                 |  3 +-
 src/rime/gear/selector.cc                  |  5 +++-
 5 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/src/rime/gear/editor.cc b/src/rime/gear/editor.cc
index 38b4a724ce..d201983820 100644
--- a/src/rime/gear/editor.cc
+++ b/src/rime/gear/editor.cc
@@ -52,7 +52,8 @@ ProcessResult Editor::ProcessKeyEvent(const KeyEvent& key_event) {
   int ch = key_event.keycode();
   Context* ctx = engine_->context();
   if (ctx->IsComposing()) {
-    auto result = KeyBindingProcessor::ProcessKeyEvent(key_event, ctx);
+    auto result = KeyBindingProcessor::ProcessKeyEvent(
+      key_event, ctx, 0, FallbackOptions::All);
     if (result != kNoop) {
       return result;
     }
diff --git a/src/rime/gear/key_binding_processor.h b/src/rime/gear/key_binding_processor.h
index 8dab13f649..3649dd40b7 100644
--- a/src/rime/gear/key_binding_processor.h
+++ b/src/rime/gear/key_binding_processor.h
@@ -19,6 +19,13 @@ class KeyBindingProcessor {
   typedef bool Handler(Context* ctx);
   using HandlerPtr = bool (T::*)(Context* ctx);
 
+  enum FallbackOptions {
+    None = 0,
+    ShiftAsControl = (1 << 0),
+    IgnoreShift = (1 << 1),
+    All = ShiftAsControl | IgnoreShift,
+  };
+
   struct ActionDef {
     const char* name;
     HandlerPtr action;
@@ -31,7 +38,8 @@ class KeyBindingProcessor {
 
   ProcessResult ProcessKeyEvent(const KeyEvent& key_event,
                                 Context* ctx,
-                                int keymap_selector = 0);
+                                int keymap_selector = 0,
+                                int fallback_options = FallbackOptions::None);
   void LoadConfig(Config* config,
                   const string& section,
                   int kemap_selector = 0);
diff --git a/src/rime/gear/key_binding_processor_impl.h b/src/rime/gear/key_binding_processor_impl.h
index 79f7f07bef..a3de636af2 100644
--- a/src/rime/gear/key_binding_processor_impl.h
+++ b/src/rime/gear/key_binding_processor_impl.h
@@ -11,30 +11,37 @@ const typename KeyBindingProcessor<T, N>::ActionDef
 
 template <class T, int N>
 ProcessResult KeyBindingProcessor<T, N>::ProcessKeyEvent(
-    const KeyEvent& key_event, Context* ctx, int keymap_selector) {
+    const KeyEvent& key_event,
+    Context* ctx,
+    int keymap_selector,
+    int fallback_options) {
   auto& keymap = get_keymap(keymap_selector);
   // exact match
   if (Accept(key_event, ctx, keymap)) {
     return kAccepted;
   }
-  // fallback: compatible modifiers
+  // try to match the fallback options
   if (key_event.ctrl() || key_event.alt()) {
     return kNoop;
   }
   if (key_event.shift()) {
-    KeyEvent shift_as_ctrl{
-      key_event.keycode(),
-      (key_event.modifier() & ~kShiftMask) | kControlMask
-    };
-    if (Accept(shift_as_ctrl, ctx, keymap)) {
-      return kAccepted;
+    if ((fallback_options & ShiftAsControl) != 0) {
+      KeyEvent shift_as_control{
+        key_event.keycode(),
+        (key_event.modifier() & ~kShiftMask) | kControlMask
+      };
+      if (Accept(shift_as_control, ctx, keymap)) {
+        return kAccepted;
+      }
     }
-    KeyEvent ignore_shift{
-      key_event.keycode(),
-      key_event.modifier() & ~kShiftMask
-    };
-    if (Accept(ignore_shift, ctx, keymap)) {
-      return kAccepted;
+    if ((fallback_options & IgnoreShift) != 0) {
+      KeyEvent ignore_shift{
+        key_event.keycode(),
+        key_event.modifier() & ~kShiftMask
+      };
+      if (Accept(ignore_shift, ctx, keymap)) {
+        return kAccepted;
+      }
     }
   }
   // not handled
diff --git a/src/rime/gear/navigator.cc b/src/rime/gear/navigator.cc
index f2e3735223..0423d2b64d 100644
--- a/src/rime/gear/navigator.cc
+++ b/src/rime/gear/navigator.cc
@@ -73,7 +73,8 @@ ProcessResult Navigator::ProcessKeyEvent(const KeyEvent& key_event) {
     return kNoop;
   TextOrientation text_orientation =
     ctx->get_option("_vertical") ? Vertical : Horizontal;
-  return KeyBindingProcessor::ProcessKeyEvent(key_event, ctx, text_orientation);
+  return KeyBindingProcessor::ProcessKeyEvent(
+    key_event, ctx, text_orientation, FallbackOptions::All);
 }
 
 bool Navigator::LeftBySyllable(Context* ctx) {
diff --git a/src/rime/gear/selector.cc b/src/rime/gear/selector.cc
index 64f6e6a905..f55719c04d 100644
--- a/src/rime/gear/selector.cc
+++ b/src/rime/gear/selector.cc
@@ -133,7 +133,10 @@ ProcessResult Selector::ProcessKeyEvent(const KeyEvent& key_event) {
   CandidateListLayout candidate_list_layout =
     is_linear_layout(ctx) ? Linear : Stacked;
   auto result = KeyBindingProcessor::ProcessKeyEvent(
-    key_event, ctx, text_orientation | candidate_list_layout);
+    key_event,
+    ctx,
+    text_orientation | candidate_list_layout,
+    FallbackOptions::None);
   if (result != kNoop) {
     return result;
   }