From 7cf74af079294beaff37faaa70830d1a04b41540 Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Sun, 25 Feb 2024 16:33:56 +0800 Subject: [PATCH 1/4] Remove dead irb_level method --- lib/irb/ext/workspaces.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/irb/ext/workspaces.rb b/lib/irb/ext/workspaces.rb index f447948fb..152f20154 100644 --- a/lib/irb/ext/workspaces.rb +++ b/lib/irb/ext/workspaces.rb @@ -6,12 +6,6 @@ module IRB # :nodoc: class Context - - # Size of the current WorkSpace stack - def irb_level - workspace_stack.size - end - # WorkSpaces in the current stack def workspaces if defined? @workspaces From fe331414636dab147c7b24df365db7825377e1e9 Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Sun, 25 Feb 2024 17:33:39 +0800 Subject: [PATCH 2/4] Restructure workspace management Currently, workspace is an attribute of IRB::Context in most use cases. But when some workspace commands are used, like `pushws` or `popws`, a workspace will be created and used along side with the original workspace attribute. This complexity is not necessary and will prevent us from expanding multi-workspace support in the future. So this commit introduces a @workspace_stack ivar to IRB::Context so IRB can have a more natural way to manage workspaces. --- lib/irb.rb | 15 +++++++------- lib/irb/command/pushws.rb | 2 +- lib/irb/context.rb | 26 +++++++++++++++-------- lib/irb/ext/change-ws.rb | 6 +++--- lib/irb/ext/eval_history.rb | 4 ++-- lib/irb/ext/use-loader.rb | 4 ++-- lib/irb/ext/workspaces.rb | 41 +++++++++++++++---------------------- test/irb/test_command.rb | 2 +- 8 files changed, 51 insertions(+), 49 deletions(-) diff --git a/lib/irb.rb b/lib/irb.rb index 559888586..0c481ff1d 100644 --- a/lib/irb.rb +++ b/lib/irb.rb @@ -933,7 +933,7 @@ def debug_break def debug_readline(binding) workspace = IRB::WorkSpace.new(binding) - context.workspace = workspace + context.replace_workspace(workspace) context.workspace.load_commands_to_main @line_no += 1 @@ -1269,12 +1269,11 @@ def suspend_name(path = nil, name = nil) # Used by the irb command +irb_load+, see IRB@IRB+Sessions for more # information. def suspend_workspace(workspace) - @context.workspace, back_workspace = workspace, @context.workspace - begin - yield back_workspace - ensure - @context.workspace = back_workspace - end + current_workspace = @context.workspace + @context.replace_workspace(workspace) + yield + ensure + @context.replace_workspace current_workspace end # Evaluates the given block using the given +input_method+ as the @@ -1534,7 +1533,7 @@ def irb(show_code: true) if debugger_irb # If we're already in a debugger session, set the workspace and irb_path for the original IRB instance - debugger_irb.context.workspace = workspace + debugger_irb.context.replace_workspace(workspace) debugger_irb.context.irb_path = irb_path # If we've started a debugger session and hit another binding.irb, we don't want to start an IRB session # instead, we want to resume the irb:rdbg session. diff --git a/lib/irb/command/pushws.rb b/lib/irb/command/pushws.rb index fadd10d6a..748efd928 100644 --- a/lib/irb/command/pushws.rb +++ b/lib/irb/command/pushws.rb @@ -15,7 +15,7 @@ class Workspaces < Base description "Show workspaces." def execute(*obj) - irb_context.workspaces.collect{|ws| ws.main} + irb_context.instance_variable_get(:@workspace_stack)[0..-2]&.collect{|ws| ws.main} end end diff --git a/lib/irb/context.rb b/lib/irb/context.rb index 964732703..60dfb9668 100644 --- a/lib/irb/context.rb +++ b/lib/irb/context.rb @@ -22,10 +22,11 @@ class Context # +other+:: uses this as InputMethod def initialize(irb, workspace = nil, input_method = nil) @irb = irb + @workspace_stack = [] if workspace - @workspace = workspace + @workspace_stack << workspace else - @workspace = WorkSpace.new + @workspace_stack << WorkSpace.new end @thread = Thread.current @@ -229,15 +230,24 @@ def history_file=(hist) IRB.conf[:HISTORY_FILE] = hist end + # Workspace in the current context. + def workspace + @workspace_stack.last + end + + # Replace the current workspace with the given +workspace+. + def replace_workspace(workspace) + @workspace_stack.pop + @workspace_stack.push(workspace) + end + # The top-level workspace, see WorkSpace#main def main - @workspace.main + workspace.main end # The toplevel workspace, see #home_workspace attr_reader :workspace_home - # WorkSpace in the current context. - attr_accessor :workspace # The current thread in this context. attr_reader :thread # The current input method. @@ -489,7 +499,7 @@ def prompting? # to #last_value. def set_last_value(value) @last_value = value - @workspace.local_variable_set :_, value + workspace.local_variable_set :_, value end # Sets the +mode+ of the prompt in this context. @@ -585,7 +595,7 @@ def evaluate(line, line_no) # :nodoc: if IRB.conf[:MEASURE] && !IRB.conf[:MEASURE_CALLBACKS].empty? last_proc = proc do - result = @workspace.evaluate(line, @eval_path, line_no) + result = workspace.evaluate(line, @eval_path, line_no) end IRB.conf[:MEASURE_CALLBACKS].inject(last_proc) do |chain, item| _name, callback, arg = item @@ -596,7 +606,7 @@ def evaluate(line, line_no) # :nodoc: end end.call else - result = @workspace.evaluate(line, @eval_path, line_no) + result = workspace.evaluate(line, @eval_path, line_no) end set_last_value(result) diff --git a/lib/irb/ext/change-ws.rb b/lib/irb/ext/change-ws.rb index ec29f7a2b..87fe03e23 100644 --- a/lib/irb/ext/change-ws.rb +++ b/lib/irb/ext/change-ws.rb @@ -12,7 +12,7 @@ def home_workspace if defined? @home_workspace @home_workspace else - @home_workspace = @workspace + @home_workspace = workspace end end @@ -25,11 +25,11 @@ def home_workspace # See IRB::WorkSpace.new for more information. def change_workspace(*_main) if _main.empty? - @workspace = home_workspace + replace_workspace(home_workspace) return main end - @workspace = WorkSpace.new(_main[0]) + replace_workspace(WorkSpace.new(_main[0])) if !(class< Date: Thu, 29 Feb 2024 00:08:57 +0800 Subject: [PATCH 3/4] Fix pushws without args --- lib/irb/ext/workspaces.rb | 2 +- test/irb/test_command.rb | 19 ++++++++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/lib/irb/ext/workspaces.rb b/lib/irb/ext/workspaces.rb index 40e69cff7..0510b2374 100644 --- a/lib/irb/ext/workspaces.rb +++ b/lib/irb/ext/workspaces.rb @@ -17,7 +17,7 @@ def push_workspace(*_main) print "No other workspace\n" else # swap the top two workspaces - previous_workspace, current_workspace = @workspace_stack.pop + previous_workspace, current_workspace = @workspace_stack.pop(2) @workspace_stack.push current_workspace, previous_workspace end else diff --git a/test/irb/test_command.rb b/test/irb/test_command.rb index 8d5c5b704..c04972de2 100644 --- a/test/irb/test_command.rb +++ b/test/irb/test_command.rb @@ -493,11 +493,13 @@ def test_cwws_returns_the_current_workspace_object class PushwsTest < WorkspaceCommandTestCase def test_pushws_switches_to_new_workspace_and_pushes_the_current_one_to_the_stack out, err = execute_lines( - "pushws #{self.class}::Foo.new\n", - "cwws.class", + "pushws #{self.class}::Foo.new", + "self.class", + "popws", + "self.class" ) assert_empty err - assert_include(out, "#{self.class}::Foo") + assert_match(/=> #{self.class}::Foo\s+=> \[\]\s+=> #{self.class}/, out) end def test_pushws_extends_the_new_workspace_with_command_bundle @@ -516,6 +518,17 @@ def test_pushws_prints_help_message_when_no_arg_is_given assert_empty err assert_match(/No other workspace/, out) end + + def test_pushws_without_argument_swaps_the_top_two_workspaces + out, err = execute_lines( + "pushws #{self.class}::Foo.new", + "self.class", + "pushws", + "self.class" + ) + assert_empty err + assert_match(/=> #{self.class}::Foo\s+=> \[#\<#{self.class}::Foo.*\>\]\s+=> #{self.class}/, out) + end end class WorkspacesTest < WorkspaceCommandTestCase From 8ea782d35f66eb096628e5f2e4d73fc86fe78aad Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Thu, 29 Feb 2024 10:49:42 +0800 Subject: [PATCH 4/4] Always display workspace stack after related commands are used --- lib/irb/command/pushws.rb | 18 +++++++++++++++++- lib/irb/ext/workspaces.rb | 14 ++------------ test/irb/test_command.rb | 37 +++++++++++++++---------------------- 3 files changed, 34 insertions(+), 35 deletions(-) diff --git a/lib/irb/command/pushws.rb b/lib/irb/command/pushws.rb index 748efd928..888fe466f 100644 --- a/lib/irb/command/pushws.rb +++ b/lib/irb/command/pushws.rb @@ -15,7 +15,23 @@ class Workspaces < Base description "Show workspaces." def execute(*obj) - irb_context.instance_variable_get(:@workspace_stack)[0..-2]&.collect{|ws| ws.main} + inspection_resuls = irb_context.instance_variable_get(:@workspace_stack).map do |ws| + truncated_inspect(ws.main) + end + + puts "[" + inspection_resuls.join(", ") + "]" + end + + private + + def truncated_inspect(obj) + obj_inspection = obj.inspect + + if obj_inspection.size > 20 + obj_inspection = obj_inspection[0, 19] + "...>" + end + + obj_inspection end end diff --git a/lib/irb/ext/workspaces.rb b/lib/irb/ext/workspaces.rb index 0510b2374..5a1edd89b 100644 --- a/lib/irb/ext/workspaces.rb +++ b/lib/irb/ext/workspaces.rb @@ -13,9 +13,7 @@ class Context # information. def push_workspace(*_main) if _main.empty? - if @workspace_stack.size == 1 - print "No other workspace\n" - else + if @workspace_stack.size > 1 # swap the top two workspaces previous_workspace, current_workspace = @workspace_stack.pop(2) @workspace_stack.push current_workspace, previous_workspace @@ -26,8 +24,6 @@ def push_workspace(*_main) main.extend ExtendCommandBundle end end - - nil end # Removes the last element from the current #workspaces stack and returns @@ -35,13 +31,7 @@ def push_workspace(*_main) # # Also, see #push_workspace. def pop_workspace - if @workspace_stack.size == 1 - print "Can't pop the last workspace on the stack\n" - else - @workspace_stack.pop - end - - nil + @workspace_stack.pop if @workspace_stack.size > 1 end end end diff --git a/test/irb/test_command.rb b/test/irb/test_command.rb index c04972de2..d6c8c534f 100644 --- a/test/irb/test_command.rb +++ b/test/irb/test_command.rb @@ -482,7 +482,8 @@ class Foo; end class CwwsTest < WorkspaceCommandTestCase def test_cwws_returns_the_current_workspace_object out, err = execute_lines( - "cwws.class", + "cwws", + "self.class" ) assert_empty err @@ -499,24 +500,26 @@ def test_pushws_switches_to_new_workspace_and_pushes_the_current_one_to_the_stac "self.class" ) assert_empty err - assert_match(/=> #{self.class}::Foo\s+=> \[\]\s+=> #{self.class}/, out) + + assert_match(/=> #{self.class}::Foo\n/, out) + assert_match(/=> #{self.class}\n$/, out) end def test_pushws_extends_the_new_workspace_with_command_bundle out, err = execute_lines( - "pushws Object.new\n", + "pushws Object.new", "self.singleton_class.ancestors" ) assert_empty err assert_include(out, "IRB::ExtendCommandBundle") end - def test_pushws_prints_help_message_when_no_arg_is_given + def test_pushws_prints_workspace_stack_when_no_arg_is_given out, err = execute_lines( - "pushws\n", + "pushws", ) assert_empty err - assert_match(/No other workspace/, out) + assert_include(out, "[#]") end def test_pushws_without_argument_swaps_the_top_two_workspaces @@ -527,30 +530,20 @@ def test_pushws_without_argument_swaps_the_top_two_workspaces "self.class" ) assert_empty err - assert_match(/=> #{self.class}::Foo\s+=> \[#\<#{self.class}::Foo.*\>\]\s+=> #{self.class}/, out) + assert_match(/=> #{self.class}::Foo\n/, out) + assert_match(/=> #{self.class}\n$/, out) end end class WorkspacesTest < WorkspaceCommandTestCase - def test_workspaces_returns_the_array_of_non_main_workspaces + def test_workspaces_returns_the_stack_of_workspaces out, err = execute_lines( "pushws #{self.class}::Foo.new\n", - "workspaces.map { |w| w.class.name }", - ) - - assert_empty err - # self.class::Foo would be the current workspace - # self.class would be the old workspace that's pushed to the stack - assert_include(out, "=> [\"#{self.class}\"]") - end - - def test_workspaces_returns_empty_array_when_no_workspaces_were_added - out, err = execute_lines( - "workspaces.map(&:to_s)", + "workspaces", ) assert_empty err - assert_include(out, "=> []") + assert_match(/\[#, #]\n/, out) end end @@ -570,7 +563,7 @@ def test_popws_prints_help_message_if_the_workspace_is_empty "popws\n", ) assert_empty err - assert_match(/Can't pop the last workspace on the stack/, out) + assert_match(/\[#\]\n/, out) end end