Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove internal-only methods from Command::Base #922

Merged
merged 2 commits into from Apr 17, 2024

Conversation

tompng
Copy link
Member

@tompng tompng commented Apr 14, 2024

I think we should hide Command#ruby_args and Command#unwrap_string_literal from custom command definition, (at least for now).

We can reduce the use of these methods. Most of the case these methods are used for backward compatibility in default commands. Hiding it, we can remove these methods in any time.

Copy link
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea 👍
I think we can even move them into a separate file so the module will not be loaded automatically either:

diff --git a/lib/irb/command/base.rb b/lib/irb/command/base.rb
index 8bf494a..b078b48 100644
--- a/lib/irb/command/base.rb
+++ b/lib/irb/command/base.rb
@@ -55,28 +55,6 @@ module IRB
       end
     end
 
-    # Internal use only, for default command's backward compatibility.
-    module RubyArgsExtractor
-      def unwrap_string_literal(str)
-        return if str.empty?
-
-        sexp = Ripper.sexp(str)
-        if sexp && sexp.size == 2 && sexp.last&.first&.first == :string_literal
-          @irb_context.workspace.binding.eval(str).to_s
-        else
-          str
-        end
-      end
-
-      def ruby_args(arg)
-        # Use throw and catch to handle arg that includes `;`
-        # For example: "1, kw: (2; 3); 4" will be parsed to [[1], { kw: 3 }]
-        catch(:EXTRACT_RUBY_ARGS) do
-          @irb_context.workspace.binding.eval "IRB::Command.extract_ruby_args #{arg}"
-        end || [[], {}]
-      end
-    end
-
     Nop = Base
   end
 
diff --git a/lib/irb/command/internal_helpers.rb b/lib/irb/command/internal_helpers.rb
new file mode 100644
index 0000000..249b5cd
--- /dev/null
+++ b/lib/irb/command/internal_helpers.rb
@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+module IRB
+  module Command
+    # Internal use only, for default command's backward compatibility.
+    module RubyArgsExtractor # :nodoc:
+      def unwrap_string_literal(str)
+        return if str.empty?
+
+        sexp = Ripper.sexp(str)
+        if sexp && sexp.size == 2 && sexp.last&.first&.first == :string_literal
+          @irb_context.workspace.binding.eval(str).to_s
+        else
+          str
+        end
+      end
+
+      def ruby_args(arg)
+        # Use throw and catch to handle arg that includes `;`
+        # For example: "1, kw: (2; 3); 4" will be parsed to [[1], { kw: 3 }]
+        catch(:EXTRACT_RUBY_ARGS) do
+          @irb_context.workspace.binding.eval "IRB::Command.extract_ruby_args #{arg}"
+        end || [[], {}]
+      end
+    end
+  end
+end
diff --git a/lib/irb/default_commands.rb b/lib/irb/default_commands.rb
index 6025b05..d680655 100644
--- a/lib/irb/default_commands.rb
+++ b/lib/irb/default_commands.rb
@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require_relative "command"
+require_relative "command/internal_helpers"
 require_relative "command/context"
 require_relative "command/exit"
 require_relative "command/force_exit"

lib/irb/command/edit.rb Outdated Show resolved Hide resolved
Command#ruby_args and Command#unwrap_string_literal are used for default command's argument backward compatibility.
Moved these methods to another module to avoid being used from custom commands.
@tompng tompng force-pushed the command_base_no_arg_extract branch from f9f4044 to 808cadb Compare April 15, 2024 16:20
lib/irb/command/edit.rb Outdated Show resolved Hide resolved
@st0012 st0012 merged commit 7405a84 into ruby:master Apr 17, 2024
29 checks passed
matzbot pushed a commit to ruby/ruby that referenced this pull request Apr 17, 2024
(ruby/irb#922)

* Remove internal-only methods from Command::Base

Command#ruby_args and Command#unwrap_string_literal are used for default command's argument backward compatibility.
Moved these methods to another module to avoid being used from custom commands.

* Update lib/irb/command/edit.rb

---------

ruby/irb@7405a841e8

Co-authored-by: Stan Lo <stan001212@gmail.com>
@tompng tompng deleted the command_base_no_arg_extract branch April 18, 2024 11:49
artur-intech pushed a commit to artur-intech/ruby that referenced this pull request Apr 26, 2024
(ruby/irb#922)

* Remove internal-only methods from Command::Base

Command#ruby_args and Command#unwrap_string_literal are used for default command's argument backward compatibility.
Moved these methods to another module to avoid being used from custom commands.

* Update lib/irb/command/edit.rb

---------

ruby/irb@7405a841e8

Co-authored-by: Stan Lo <stan001212@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants