Use color name instead of code (integer) in dialog color APIs

As pointed out in the
[comment](ruby#413 (comment)),
the code is actually a control sequence and not only for colors.

To make the dialog color APIs safer to use, we should restrict its
usages and extract away the bg/fg concept from the input.

So in this commit, I made these changes:

1. The dialog_*_bg/fg_color APIs only takes and returns color names (symbol):
  - :black
  - :red
  - :green
  - :yellow
  - :blue
  - :magenta
  - :cyan
  - :white
2. Add additional dialog_*_bg/fg_color_sequence APIs to access the raw code.

lib/reline.rb

@@ -46,6 +46,21 @@ def match?(other)
     keyword_init: true
   )
 
+  DIALOG_COLOR_APIS = [
+    :dialog_default_bg_color,
+    :dialog_default_bg_color_sequence,
+    :dialog_default_bg_color=,
+    :dialog_default_fg_color,
+    :dialog_default_fg_color_sequence,
+    :dialog_default_fg_color=,
+    :dialog_pointer_bg_color,
+    :dialog_pointer_bg_color_sequence,
+    :dialog_pointer_bg_color=,
+    :dialog_pointer_fg_color,
+    :dialog_pointer_fg_color_sequence,
+    :dialog_pointer_fg_color=
+  ]
+
   class Core
     ATTR_READER_NAMES = %i(
       completion_append_character
@@ -73,14 +88,7 @@ class Core
     def_delegators :config,
       :autocompletion,
       :autocompletion=,
-      :dialog_default_bg_color,
-      :dialog_default_bg_color=,
-      :dialog_default_fg_color,
-      :dialog_default_fg_color=,
-      :dialog_pointer_bg_color,
-      :dialog_pointer_bg_color=,
-      :dialog_pointer_fg_color,
-      :dialog_pointer_fg_color=
+      *DIALOG_COLOR_APIS
 
     def initialize
       self.output = STDOUT
@@ -264,10 +272,10 @@ def get_screen_size
         contents: result,
         scrollbar: true,
         height: 15,
-        bg_color: config.dialog_default_bg_color,
-        pointer_bg_color: config.dialog_pointer_bg_color,
-        fg_color: config.dialog_default_fg_color,
-        pointer_fg_color: config.dialog_pointer_fg_color
+        bg_color: config.dialog_default_bg_color_sequence,
+        pointer_bg_color: config.dialog_pointer_bg_color_sequence,
+        fg_color: config.dialog_default_fg_color_sequence,
+        pointer_fg_color: config.dialog_pointer_fg_color_sequence
       )
     }
     Reline::DEFAULT_DIALOG_CONTEXT = Array.new
@@ -553,10 +561,7 @@ def self.insert_text(*args, &block)
   def_single_delegators :core, :add_dialog_proc
   def_single_delegators :core, :dialog_proc
   def_single_delegators :core, :autocompletion, :autocompletion=
-  def_single_delegators :core, :dialog_default_bg_color, :dialog_default_bg_color=
-  def_single_delegators :core, :dialog_pointer_bg_color, :dialog_pointer_bg_color=
-  def_single_delegators :core, :dialog_default_fg_color, :dialog_default_fg_color=
-  def_single_delegators :core, :dialog_pointer_fg_color, :dialog_pointer_fg_color=
+  def_single_delegators :core, *DIALOG_COLOR_APIS
 
   def_single_delegators :core, :readmultiline
   def_instance_delegators self, :readmultiline
@@ -579,10 +584,10 @@ def self.core
       core.filename_quote_characters = ""
       core.special_prefixes = ""
       core.add_dialog_proc(:autocomplete, Reline::DEFAULT_DIALOG_PROC_AUTOCOMPLETE, Reline::DEFAULT_DIALOG_CONTEXT)
-      core.dialog_default_bg_color = 46 # Cyan
-      core.dialog_default_fg_color = 37 # White
-      core.dialog_pointer_bg_color = 45 # Magenta
-      core.dialog_pointer_fg_color = 37 # White
+      core.dialog_default_bg_color = :cyan
+      core.dialog_default_fg_color = :white
+      core.dialog_pointer_bg_color = :magenta
+      core.dialog_pointer_fg_color = :white
     }
   end
 

lib/reline/config.rb

@@ -45,13 +45,11 @@ class InvalidInputrc < RuntimeError
     attr_accessor v
   end
 
-  attr_accessor(
-    :autocompletion,
-    :dialog_default_bg_color,
-    :dialog_default_fg_color,
-    :dialog_pointer_bg_color,
-    :dialog_pointer_fg_color,
-  )
+  attr_accessor :autocompletion
+  attr_reader :dialog_default_bg_color_sequence,
+    :dialog_default_fg_color_sequence,
+    :dialog_pointer_bg_color_sequence,
+    :dialog_pointer_fg_color_sequence
 
   def initialize
     @additional_key_bindings = {} # from inputrc
@@ -77,10 +75,10 @@ def initialize
     @test_mode = false
     @autocompletion = false
     @convert_meta = true if seven_bit_encoding?(Reline::IOGate.encoding)
-    @dialog_default_bg_color = nil
-    @dialog_pointer_bg_color = nil
-    @dialog_default_fg_color = nil
-    @dialog_pointer_fg_color = nil
+    @dialog_default_bg_color_sequence = nil
+    @dialog_pointer_bg_color_sequence = nil
+    @dialog_default_fg_color_sequence = nil
+    @dialog_pointer_fg_color_sequence = nil
   end
 
   def reset
@@ -106,6 +104,65 @@ def editing_mode_is?(*val)
     (val.respond_to?(:any?) ? val : [val]).any?(@editing_mode_label)
   end
 
+  def dialog_default_bg_color=(color)
+    @dialog_default_bg_color_sequence = dialog_color_to_code(:bg, color)
+  end
+
+  def dialog_default_fg_color=(color)
+    @dialog_default_fg_color_sequence = dialog_color_to_code(:fg, color)
+  end
+
+  def dialog_pointer_bg_color=(color)
+    @dialog_pointer_bg_color_sequence = dialog_color_to_code(:bg, color)
+  end
+
+  def dialog_pointer_fg_color=(color)
+    @dialog_pointer_fg_color_sequence = dialog_color_to_code(:fg, color)
+  end
+
+  def dialog_default_bg_color
+    dialog_code_to_color(:bg, @dialog_default_bg_color_sequence)
+  end
+
+  def dialog_default_fg_color
+    dialog_code_to_color(:fg, @dialog_default_fg_color_sequence)
+  end
+
+  def dialog_pointer_bg_color
+    dialog_code_to_color(:bg, @dialog_pointer_bg_color_sequence)
+  end
+
+  def dialog_pointer_fg_color
+    dialog_code_to_color(:fg, @dialog_pointer_fg_color_sequence)
+  end
+
+  COLORS = [
+    :black,
+    :red,
+    :green,
+    :yellow,
+    :blue,
+    :magenta,
+    :cyan,
+    :white
+  ].freeze
+
+  private def dialog_color_to_code(type, color)
+    base = type == :bg ? 40 : 30
+    c = COLORS.index(color.to_sym)
+
+    if c
+      base + c
+    else
+      raise ArgumentError.new("Unknown color: #{color}.\nAvailable colors: #{COLORS.join(", ")}")
+    end
+  end
+
+  private def dialog_code_to_color(type, code)
+    base = type == :bg ? 40 : 30
+    COLORS[code - base]
+  end
+
   def keymap
     @key_actors[@keymap_label]
   end
@@ -339,13 +396,13 @@ def bind_variable(name, value)
     when 'emacs-mode-string'
       @emacs_mode_string = retrieve_string(value)
     when 'dialog-default-bg-color'
-      @dialog_default_bg_color = value.to_i
-    when 'dialog-pointer-bg-color'
-      @dialog_pointer_bg_color = value.to_i
+      self.dialog_default_bg_color = value
     when 'dialog-default-fg-color'
-      @dialog_default_fg_color = value.to_i
+      self.dialog_default_fg_color = value
+    when 'dialog-pointer-bg-color'
+      self.dialog_pointer_bg_color = value
     when 'dialog-pointer-fg-color'
-      @dialog_pointer_fg_color = value.to_i
+      self.dialog_pointer_fg_color = value
     when *VARIABLE_NAMES then
       variable_name = :"@#{name.tr(?-, ?_)}"
       instance_variable_set(variable_name, value.nil? || value == '1' || value == 'on')

test/reline/test_config.rb

@@ -411,16 +411,16 @@ def test_relative_xdg_config_home
 
   def test_dialog_configurations
     @config.read_lines(<<~LINES.lines)
-      set dialog-default-bg-color 1
-      set dialog-pointer-bg-color 2
-      set dialog-default-fg-color 3
-      set dialog-pointer-fg-color 4
+      set dialog-default-bg-color white
+      set dialog-pointer-bg-color black
+      set dialog-default-fg-color cyan
+      set dialog-pointer-fg-color magenta
     LINES
 
-    assert_equal 1, @config.dialog_default_bg_color
-    assert_equal 2, @config.dialog_pointer_bg_color
-    assert_equal 3, @config.dialog_default_fg_color
-    assert_equal 4, @config.dialog_pointer_fg_color
+    assert_equal :white, @config.dialog_default_bg_color
+    assert_equal :black, @config.dialog_pointer_bg_color
+    assert_equal :cyan, @config.dialog_default_fg_color
+    assert_equal :magenta, @config.dialog_pointer_fg_color
   end
 end
 

test/reline/test_reline.rb

@@ -48,19 +48,31 @@ def test_completion_append_character
 
   def test_dialog_color_configuration
     # defaults
-    assert_equal(46, Reline.dialog_default_bg_color)
-    assert_equal(37, Reline.dialog_default_fg_color)
-    assert_equal(45, Reline.dialog_pointer_bg_color)
-    assert_equal(37, Reline.dialog_pointer_fg_color)
-
-    Reline.dialog_default_bg_color = 40
-    assert_equal(40, Reline.dialog_default_bg_color)
-    Reline.dialog_default_fg_color = 47
-    assert_equal(47, Reline.dialog_default_fg_color)
-    Reline.dialog_pointer_bg_color = 37
-    assert_equal(37, Reline.dialog_pointer_bg_color)
-    Reline.dialog_pointer_fg_color = 30
-    assert_equal(30, Reline.dialog_pointer_fg_color)
+    assert_equal(:cyan, Reline.dialog_default_bg_color)
+    assert_equal(:white, Reline.dialog_default_fg_color)
+    assert_equal(:magenta, Reline.dialog_pointer_bg_color)
+    assert_equal(:white, Reline.dialog_pointer_fg_color)
+
+    Reline.dialog_default_bg_color = :black
+    assert_equal(:black, Reline.dialog_default_bg_color)
+    assert_equal(40, Reline.dialog_default_bg_color_sequence)
+
+    Reline.dialog_default_fg_color = :white
+    assert_equal(:white, Reline.dialog_default_fg_color)
+    assert_equal(37, Reline.dialog_default_fg_color_sequence)
+
+    Reline.dialog_pointer_bg_color = :white
+    assert_equal(:white, Reline.dialog_pointer_bg_color)
+    assert_equal(47, Reline.dialog_pointer_bg_color_sequence)
+
+    Reline.dialog_pointer_fg_color = :black
+    assert_equal(:black, Reline.dialog_pointer_fg_color)
+    assert_equal(30, Reline.dialog_pointer_fg_color_sequence)
+
+    # test value validation
+    assert_raise(ArgumentError) do
+      Reline.dialog_pointer_fg_color = :foo
+    end
   end
 
   def test_basic_word_break_characters