Improve key binding match/matching check (#709)

* Improve key binding match/matching check

* Rename key_actors to default_key_bindings

* Make key_stroke.expand always return a value

* Update add_default_key_binding to use a add_default_key_binding_by_keymap internally

Co-authored-by: Stan Lo <stan001212@gmail.com>

---------

Co-authored-by: Stan Lo <stan001212@gmail.com>

lib/reline.rb

@@ -19,20 +19,10 @@ module Reline
   class ConfigEncodingConversionError < StandardError; end
 
   Key = Struct.new(:char, :combined_char, :with_meta) do
-    def match?(other)
-      case other
-      when Reline::Key
-        (other.char.nil? or char.nil? or char == other.char) and
-        (other.combined_char.nil? or combined_char.nil? or combined_char == other.combined_char) and
-        (other.with_meta.nil? or with_meta.nil? or with_meta == other.with_meta)
-      when Integer, Symbol
-        (combined_char and combined_char == other) or
-        (combined_char.nil? and char and char == other)
-      else
-        false
-      end
+    # For dialog_proc `key.match?(dialog.name)`
+    def match?(sym)
+      combined_char.is_a?(Symbol) && combined_char == sym
     end
-    alias_method :==, :match?
   end
   CursorPos = Struct.new(:x, :y)
   DialogRenderInfo = Struct.new(
@@ -400,9 +390,8 @@ def readline(prompt = '', add_hist = false)
         end
         case result
         when :matched
-          expanded = key_stroke.expand(buffer).map{ |expanded_c|
-            Reline::Key.new(expanded_c, expanded_c, false)
-          }
+          expanded, rest_bytes = key_stroke.expand(buffer)
+          rest_bytes.reverse_each { |c| io_gate.ungetc(c) }
           block.(expanded)
           break
         when :matching
@@ -416,9 +405,8 @@ def readline(prompt = '', add_hist = false)
           if buffer.size == 1 and c == "\e".ord
             read_escaped_key(keyseq_timeout, c, block)
           else
-            expanded = buffer.map{ |expanded_c|
-              Reline::Key.new(expanded_c, expanded_c, false)
-            }
+            expanded, rest_bytes = key_stroke.expand(buffer)
+            rest_bytes.reverse_each { |c| io_gate.ungetc(c) }
             block.(expanded)
           end
           break
@@ -442,9 +430,8 @@ def readline(prompt = '', add_hist = false)
           return :next
         when :matched
           buffer << succ_c
-          expanded = key_stroke.expand(buffer).map{ |expanded_c|
-            Reline::Key.new(expanded_c, expanded_c, false)
-          }
+          expanded, rest_bytes = key_stroke.expand(buffer)
+          rest_bytes.reverse_each { |c| io_gate.ungetc(c) }
           block.(expanded)
           return :break
         end

lib/reline/config.rb

@@ -29,18 +29,20 @@ class InvalidInputrc < RuntimeError
   attr_accessor :autocompletion
 
   def initialize
-    @additional_key_bindings = {} # from inputrc
-    @additional_key_bindings[:emacs] = {}
-    @additional_key_bindings[:vi_insert] = {}
-    @additional_key_bindings[:vi_command] = {}
-    @oneshot_key_bindings = {}
+    @additional_key_bindings = { # from inputrc
+      emacs: Reline::KeyActor::Base.new,
+      vi_insert: Reline::KeyActor::Base.new,
+      vi_command: Reline::KeyActor::Base.new
+    }
+    @oneshot_key_bindings = Reline::KeyActor::Base.new
     @editing_mode_label = :emacs
     @keymap_label = :emacs
     @keymap_prefix = []
-    @key_actors = {}
-    @key_actors[:emacs] = Reline::KeyActor::Emacs.new
-    @key_actors[:vi_insert] = Reline::KeyActor::ViInsert.new
-    @key_actors[:vi_command] = Reline::KeyActor::ViCommand.new
+    @default_key_bindings = {
+      emacs: Reline::KeyActor::Base.new(Reline::KeyActor::EMACS_MAPPING),
+      vi_insert: Reline::KeyActor::Base.new(Reline::KeyActor::VI_INSERT_MAPPING),
+      vi_command: Reline::KeyActor::Base.new(Reline::KeyActor::VI_COMMAND_MAPPING)
+    }
     @vi_cmd_mode_string = '(cmd)'
     @vi_ins_mode_string = '(ins)'
     @emacs_mode_string = '@'
@@ -62,7 +64,7 @@ def reset
   end
 
   def editing_mode
-    @key_actors[@editing_mode_label]
+    @default_key_bindings[@editing_mode_label]
   end
 
   def editing_mode=(val)
@@ -74,7 +76,7 @@ def editing_mode_is?(*val)
   end
 
   def keymap
-    @key_actors[@keymap_label]
+    @default_key_bindings[@keymap_label]
   end
 
   def loaded?
@@ -133,26 +135,26 @@ def read(file = nil)
 
   def key_bindings
     # The key bindings for each editing mode will be overwritten by the user-defined ones.
-    kb = @key_actors[@editing_mode_label].default_key_bindings.dup
-    kb.merge!(@additional_key_bindings[@editing_mode_label])
-    kb.merge!(@oneshot_key_bindings)
-    kb
+    Reline::KeyActor::Composite.new([@oneshot_key_bindings, @additional_key_bindings[@editing_mode_label], @default_key_bindings[@editing_mode_label]])
   end
 
   def add_oneshot_key_binding(keystroke, target)
-    @oneshot_key_bindings[keystroke] = target
+    # IRB sets invalid keystroke [Reline::Key]. We should ignore it.
+    return unless keystroke.all? { |c| c.is_a?(Integer) }
+
+    @oneshot_key_bindings.add(keystroke, target)
   end
 
   def reset_oneshot_key_bindings
     @oneshot_key_bindings.clear
   end
 
   def add_default_key_binding_by_keymap(keymap, keystroke, target)
-    @key_actors[keymap].default_key_bindings[keystroke] = target
+    @default_key_bindings[keymap].add(keystroke, target)
   end
 
   def add_default_key_binding(keystroke, target)
-    @key_actors[@keymap_label].default_key_bindings[keystroke] = target
+    add_default_key_binding_by_keymap(@keymap_label, keystroke, target)
   end
 
   def read_lines(lines, file = nil)
@@ -192,7 +194,7 @@ def read_lines(lines, file = nil)
         func_name = func_name.split.first
         keystroke, func = bind_key(key, func_name)
         next unless keystroke
-        @additional_key_bindings[@keymap_label][@keymap_prefix + keystroke] = func
+        @additional_key_bindings[@keymap_label].add(@keymap_prefix + keystroke, func)
       end
     end
     unless if_stack.empty?

lib/reline/key_actor.rb

@@ -2,6 +2,7 @@ module Reline::KeyActor
 end
 
 require 'reline/key_actor/base'
+require 'reline/key_actor/composite'
 require 'reline/key_actor/emacs'
 require 'reline/key_actor/vi_command'
 require 'reline/key_actor/vi_insert'

lib/reline/key_actor/base.rb

@@ -1,15 +1,31 @@
 class Reline::KeyActor::Base
-  MAPPING = Array.new(256)
+  def initialize(mapping = [])
+    @mapping = mapping
+    @matching_bytes = {}
+    @key_bindings = {}
+  end
 
   def get_method(key)
-    self.class::MAPPING[key]
+    @mapping[key]
+  end
+
+  def add(key, func)
+    (1...key.size).each do |size|
+      @matching_bytes[key.take(size)] = true
+    end
+    @key_bindings[key] = func
+  end
+
+  def matching?(key)
+    @matching_bytes[key]
   end
 
-  def initialize
-    @default_key_bindings = {}
+  def get(key)
+    @key_bindings[key]
   end
 
-  def default_key_bindings
-    @default_key_bindings
+  def clear
+    @matching_bytes.clear
+    @key_bindings.clear
   end
 end

lib/reline/key_actor/composite.rb

@@ -0,0 +1,17 @@
+class Reline::KeyActor::Composite
+  def initialize(key_actors)
+    @key_actors = key_actors
+  end
+
+  def matching?(key)
+    @key_actors.any? { |key_actor| key_actor.matching?(key) }
+  end
+
+  def get(key)
+    @key_actors.each do |key_actor|
+      func = key_actor.get(key)
+      return func if func
+    end
+    nil
+  end
+end

lib/reline/key_actor/emacs.rb

@@ -1,5 +1,5 @@
-class Reline::KeyActor::Emacs < Reline::KeyActor::Base
-  MAPPING = [
+module Reline::KeyActor
+  EMACS_MAPPING = [
     #   0 ^@
     :em_set_mark,
     #   1 ^A

lib/reline/key_actor/vi_command.rb

@@ -1,5 +1,5 @@
-class Reline::KeyActor::ViCommand < Reline::KeyActor::Base
-  MAPPING = [
+module Reline::KeyActor
+  VI_COMMAND_MAPPING = [
     #   0 ^@
     :ed_unassigned,
     #   1 ^A

lib/reline/key_actor/vi_insert.rb

@@ -1,5 +1,5 @@
-class Reline::KeyActor::ViInsert < Reline::KeyActor::Base
-  MAPPING = [
+module Reline::KeyActor
+  VI_INSERT_MAPPING = [
     #   0 ^@
     :ed_unassigned,
     #   1 ^A