Command registration should take both strings and symbols as names (#932

)

This will save users some heads scratching when they try to register a
command with a string name and found that it doesn't work.

I also rewrote converted custom command tests into integration tests to
make test setup/cleanup easier.

lib/irb/command.rb

@@ -16,7 +16,7 @@ class << self
       # Registers a command with the given name.
       # Aliasing is intentionally not supported at the moment.
       def register(name, command_class)
-        @commands[name] = [command_class, []]
+        @commands[name.to_sym] = [command_class, []]
       end
     end
   end

lib/irb/default_commands.rb

@@ -39,7 +39,7 @@ class << self
       # This API is for IRB's internal use only and may change at any time.
       # Please do NOT use it.
       def _register_with_aliases(name, command_class, *aliases)
-        @commands[name] = [command_class, aliases]
+        @commands[name.to_sym] = [command_class, aliases]
       end
 
       def all_commands_info

test/irb/command/test_custom_command.rb

@@ -0,0 +1,127 @@
+# frozen_string_literal: true
+require "irb"
+
+require_relative "../helper"
+
+module TestIRB
+  class CustomCommandIntegrationTest < TestIRB::IntegrationTestCase
+    def test_command_regsitration_can_happen_after_irb_require
+      write_ruby <<~RUBY
+        require "irb"
+        require "irb/command"
+
+        class PrintCommand < IRB::Command::Base
+          category 'CommandTest'
+          description 'print_command'
+          def execute(*)
+            puts "Hello from PrintCommand"
+            nil
+          end
+        end
+
+        IRB::Command.register(:print!, PrintCommand)
+
+        binding.irb
+      RUBY
+
+      output = run_ruby_file do
+        type "print!\n"
+        type "exit"
+      end
+
+      assert_include(output, "Hello from PrintCommand")
+    end
+
+    def test_command_regsitration_accepts_string_too
+      write_ruby <<~RUBY
+        require "irb/command"
+
+        class PrintCommand < IRB::Command::Base
+          category 'CommandTest'
+          description 'print_command'
+          def execute(*)
+            puts "Hello from PrintCommand"
+            nil
+          end
+        end
+
+        IRB::Command.register("print!", PrintCommand)
+
+        binding.irb
+      RUBY
+
+      output = run_ruby_file do
+        type "print!\n"
+        type "exit"
+      end
+
+      assert_include(output, "Hello from PrintCommand")
+    end
+
+    def test_arguments_propogation
+      write_ruby <<~RUBY
+        require "irb/command"
+
+        class PrintArgCommand < IRB::Command::Base
+          category 'CommandTest'
+          description 'print_command_arg'
+          def execute(arg)
+            $nth_execution ||= 0
+            puts "\#{$nth_execution} arg=\#{arg.inspect}"
+            $nth_execution += 1
+            nil
+          end
+        end
+
+        IRB::Command.register(:print_arg, PrintArgCommand)
+
+        binding.irb
+      RUBY
+
+      output = run_ruby_file do
+        type "print_arg\n"
+        type "print_arg  \n"
+        type "print_arg a r  g\n"
+        type "print_arg  a r  g  \n"
+        type "exit"
+      end
+
+      assert_include(output, "0 arg=\"\"")
+      assert_include(output, "1 arg=\"\"")
+      assert_include(output, "2 arg=\"a r  g\"")
+      assert_include(output, "3 arg=\"a r  g\"")
+    end
+
+    def test_def_extend_command_still_works
+      write_ruby <<~RUBY
+        require "irb"
+
+        class FooBarCommand < IRB::Command::Base
+          category 'FooBarCategory'
+          description 'foobar_description'
+          def execute(*)
+            $nth_execution ||= 1
+            puts "\#{$nth_execution} FooBar executed"
+            $nth_execution += 1
+            nil
+          end
+        end
+
+        IRB::ExtendCommandBundle.def_extend_command(:foobar, FooBarCommand, nil, [:fbalias, IRB::Command::OVERRIDE_ALL])
+
+        binding.irb
+      RUBY
+
+      output = run_ruby_file do
+        type "foobar"
+        type "fbalias"
+        type "help foobar"
+        type "exit"
+      end
+
+      assert_include(output, "1 FooBar executed")
+      assert_include(output, "2 FooBar executed")
+      assert_include(output, "foobar_description")
+    end
+  end
+end

test/irb/test_command.rb

@@ -210,76 +210,6 @@ def test_irb_info_lang
     end
   end
 
-  class CustomCommandTestCase < CommandTestCase
-    def setup
-      @commands_backup = IRB::Command.commands
-      IRB::Command.class_variable_set(:@@command_override_policies, nil)
-    end
-
-    def teardown
-      IRB::Command.class_variable_set(:@@command_override_policies, nil)
-      IRB::Command.instance_variable_set(:@commands, @commands_backup)
-    end
-  end
-
-  class CommandArgTest < CustomCommandTestCase
-    class PrintArgCommand < IRB::Command::Base
-      category 'CommandTest'
-      description 'print_command_arg'
-      def execute(arg)
-        puts "arg=#{arg.inspect}"
-      end
-    end
-
-    def test_arg
-      IRB::Command._register_with_aliases(:print_arg, PrintArgCommand, [:pa, IRB::Command::OVERRIDE_ALL])
-      out, err = execute_lines("print_arg\n")
-      assert_empty err
-      assert_include(out, 'arg=""')
-
-      out, err = execute_lines("print_arg  \n")
-      assert_empty err
-      assert_include(out, 'arg=""')
-
-      out, err = execute_lines("print_arg a r  g\n")
-      assert_empty err
-      assert_include(out, 'arg="a r  g"')
-
-      out, err = execute_lines("print_arg  a r  g  \n")
-      assert_empty err
-      assert_include(out, 'arg="a r  g"')
-
-      out, err = execute_lines("pa  a r  g  \n")
-      assert_empty err
-      assert_include(out, 'arg="a r  g"')
-    end
-  end
-
-  class ExtendCommandBundleCompatibilityTest < CustomCommandTestCase
-    class FooBarCommand < IRB::Command::Base
-      category 'FooBarCategory'
-      description 'foobar_description'
-      def execute(_arg)
-        puts "FooBar executed"
-      end
-    end
-
-    def test_def_extend_command
-      IRB::ExtendCommandBundle.def_extend_command(:foobar, FooBarCommand, nil, [:fbalias, IRB::Command::OVERRIDE_ALL])
-      out, err = execute_lines("foobar\n")
-      assert_empty err
-      assert_include(out, "FooBar executed")
-
-      out, err = execute_lines("fbalias\n")
-      assert_empty err
-      assert_include(out, "FooBar executed")
-
-      out, err = execute_lines("show_cmds\n")
-      assert_include(out, "FooBarCategory")
-      assert_include(out, "foobar_description")
-    end
-  end
-
   class MeasureTest < CommandTestCase
     def test_measure
       conf = {