From 22b016b0f1a7809d3ce2c9c6d283315e71d75017 Mon Sep 17 00:00:00 2001
From: Stan Lo <stan001212@gmail.com>
Date: Mon, 17 Jun 2024 12:53:05 +0100
Subject: [PATCH] Return only commands when completing help command's argument

The command only takes command names as arguments, so we should only
return command names as candidates.

This will help users find a command faster as completion will be
another useful hint too.
---
 lib/irb/completion.rb           | 38 +++++++++++++++++++++++++--------
 test/irb/test_completion.rb     | 10 +++++++--
 test/irb/test_type_completor.rb |  5 +++++
 3 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/lib/irb/completion.rb b/lib/irb/completion.rb
index a3d89373c..7f102dcdf 100644
--- a/lib/irb/completion.rb
+++ b/lib/irb/completion.rb
@@ -33,6 +33,8 @@ def defined? do
       yield
     ]
 
+    HELP_COMMAND_PREPOSING = /\Ahelp\s+/
+
     def completion_candidates(preposing, target, postposing, bind:)
       raise NotImplementedError
     end
@@ -86,8 +88,8 @@ def retrieve_files_to_require_from_load_path
         )
     end
 
-    def command_completions(preposing, target)
-      if preposing.empty? && !target.empty?
+    def command_candidates(target)
+      if !target.empty?
         IRB::Command.command_names.select { _1.start_with?(target) }
       else
         []
@@ -111,8 +113,18 @@ def inspect
     end
 
     def completion_candidates(preposing, target, _postposing, bind:)
-      commands = command_completions(preposing, target)
+      # When completing the argument of `help` command, only commands should be candidates
+      return command_candidates(target) if preposing.match?(HELP_COMMAND_PREPOSING)
+
+      commands = if preposing.empty?
+        command_candidates(target)
+      # It doesn't make sense to propose commands with other preposing
+      else
+        []
+      end
+
       result = ReplTypeCompletor.analyze(preposing + target, binding: bind, filename: @context.irb_path)
+
       return commands unless result
 
       commands | result.completion_candidates.map { target + _1 }
@@ -187,12 +199,20 @@ def complete_require_path(target, preposing, postposing)
     end
 
     def completion_candidates(preposing, target, postposing, bind:)
-      if preposing && postposing
-        result = complete_require_path(target, preposing, postposing)
-        return result if result
+      if result = complete_require_path(target, preposing, postposing)
+        return result
       end
-      commands = command_completions(preposing || '', target)
-      commands | retrieve_completion_data(target, bind: bind, doc_namespace: false).compact.map{ |i| i.encode(Encoding.default_external) }
+
+      commands = command_candidates(target)
+
+      # When completing the argument of `help` command, only commands should be candidates
+      return commands if preposing.match?(HELP_COMMAND_PREPOSING)
+
+      # It doesn't make sense to propose commands with other preposing
+      commands = [] unless preposing.empty?
+
+      completion_data = retrieve_completion_data(target, bind: bind, doc_namespace: false).compact.map{ |i| i.encode(Encoding.default_external) }
+      commands | completion_data
     end
 
     def doc_namespace(_preposing, matched, _postposing, bind:)
@@ -470,7 +490,7 @@ def retrieve_completion_data(input, bind: IRB.conf[:MAIN_CONTEXT].workspace.bind
       end
     end
     CompletionProc = ->(target, preposing = nil, postposing = nil) {
-      regexp_completor.completion_candidates(preposing, target, postposing, bind: IRB.conf[:MAIN_CONTEXT].workspace.binding)
+      regexp_completor.completion_candidates(preposing || '', target, postposing || '', bind: IRB.conf[:MAIN_CONTEXT].workspace.binding)
     }
   end
   deprecate_constant :InputCompletor
diff --git a/test/irb/test_completion.rb b/test/irb/test_completion.rb
index 5fe7952b3..c9a0eafa3 100644
--- a/test/irb/test_completion.rb
+++ b/test/irb/test_completion.rb
@@ -16,8 +16,14 @@ def doc_namespace(target, bind)
 
     class CommandCompletionTest < CompletionTest
       def test_command_completion
-        assert_include(IRB::RegexpCompletor.new.completion_candidates('', 'show_s', '', bind: binding), 'show_source')
-        assert_not_include(IRB::RegexpCompletor.new.completion_candidates(';', 'show_s', '', bind: binding), 'show_source')
+        completor = IRB::RegexpCompletor.new
+        binding.eval("some_var = 1")
+        # completion for help command's argument should only include command names
+        assert_include(completor.completion_candidates('help ', 's', '', bind: binding), 'show_source')
+        assert_not_include(completor.completion_candidates('help ', 's', '', bind: binding), 'some_var')
+
+        assert_include(completor.completion_candidates('', 'show_s', '', bind: binding), 'show_source')
+        assert_not_include(completor.completion_candidates(';', 'show_s', '', bind: binding), 'show_source')
       end
     end
 
diff --git a/test/irb/test_type_completor.rb b/test/irb/test_type_completor.rb
index 5ed8988b3..412d7c696 100644
--- a/test/irb/test_type_completor.rb
+++ b/test/irb/test_type_completor.rb
@@ -56,6 +56,11 @@ def test_empty_completion
     end
 
     def test_command_completion
+      binding.eval("some_var = 1")
+      # completion for help command's argument should only include command names
+      assert_include(@completor.completion_candidates('help ', 's', '', bind: binding), 'show_source')
+      assert_not_include(@completor.completion_candidates('help ', 's', '', bind: binding), 'some_var')
+
       assert_include(@completor.completion_candidates('', 'show_s', '', bind: binding), 'show_source')
       assert_not_include(@completor.completion_candidates(';', 'show_s', '', bind: binding), 'show_source')
     end