From 0fef0ae160c498dd7295151eaaa5c7ef9b3c32a2 Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Wed, 9 Aug 2023 15:57:47 +0100 Subject: [PATCH] Fix nested IRB sessions' history saving (#652) 1. Dynamically including `HistorySavingAbility` makes things unnecessarily complicated and should be avoided. 2. Because both `Reline` and `Readline` use a single `HISTORY` constant to store history data. When nesting IRB sessions, only the first IRB session should handle history loading and saving so we can avoid duplicating history. 3. History saving callback should NOT be stored in `IRB.conf` as it's recreated every time `IRB.setup` is called, which would happen when nesting IRB sessions. --- lib/irb.rb | 8 ++++ lib/irb/context.rb | 10 ----- lib/irb/history.rb | 6 +-- lib/irb/input-method.rb | 12 ++---- test/irb/test_history.rb | 88 +++++++++++++++++++++++++++++++++++++++- 5 files changed, 101 insertions(+), 23 deletions(-) diff --git a/lib/irb.rb b/lib/irb.rb index 1f86a0f38..839115d64 100644 --- a/lib/irb.rb +++ b/lib/irb.rb @@ -482,9 +482,16 @@ def debug_break end def run(conf = IRB.conf) + in_nested_session = !!conf[:MAIN_CONTEXT] conf[:IRB_RC].call(context) if conf[:IRB_RC] conf[:MAIN_CONTEXT] = context + save_history = !in_nested_session && conf[:SAVE_HISTORY] && context.io.support_history_saving? + + if save_history + context.io.load_history + end + prev_trap = trap("SIGINT") do signal_handle end @@ -496,6 +503,7 @@ def run(conf = IRB.conf) ensure trap("SIGINT", prev_trap) conf[:AT_EXIT].each{|hook| hook.call} + context.io.save_history if save_history end end diff --git a/lib/irb/context.rb b/lib/irb/context.rb index bf5b4c570..18125ff6f 100644 --- a/lib/irb/context.rb +++ b/lib/irb/context.rb @@ -8,7 +8,6 @@ require_relative "inspector" require_relative "input-method" require_relative "output-method" -require_relative "history" module IRB # A class that wraps the current state of the irb session, including the @@ -130,8 +129,6 @@ def initialize(irb, workspace = nil, input_method = nil) else @io = input_method end - self.save_history = IRB.conf[:SAVE_HISTORY] if IRB.conf[:SAVE_HISTORY] - @extra_doc_dirs = IRB.conf[:EXTRA_DOC_DIRS] @echo = IRB.conf[:ECHO] @@ -154,13 +151,6 @@ def initialize(irb, workspace = nil, input_method = nil) def save_history=(val) IRB.conf[:SAVE_HISTORY] = val - - if val - context = (IRB.conf[:MAIN_CONTEXT] || self) - if context.io.support_history_saving? && !context.io.singleton_class.include?(HistorySavingAbility) - context.io.extend(HistorySavingAbility) - end - end end def save_history diff --git a/lib/irb/history.rb b/lib/irb/history.rb index e18ff516b..516890ac0 100644 --- a/lib/irb/history.rb +++ b/lib/irb/history.rb @@ -1,9 +1,7 @@ module IRB module HistorySavingAbility # :nodoc: - def HistorySavingAbility.extended(obj) - IRB.conf[:AT_EXIT].push proc{obj.save_history} - obj.load_history - obj + def support_history_saving? + true end def load_history diff --git a/lib/irb/input-method.rb b/lib/irb/input-method.rb index 4dc43abba..245b93566 100644 --- a/lib/irb/input-method.rb +++ b/lib/irb/input-method.rb @@ -5,6 +5,7 @@ # require_relative 'completion' +require_relative "history" require 'io/console' require 'reline' @@ -167,6 +168,8 @@ def self.initialize_readline include ::Readline end + include HistorySavingAbility + # Creates a new input method object using Readline def initialize self.class.initialize_readline @@ -219,10 +222,6 @@ def readable_after_eof? true end - def support_history_saving? - true - end - # Returns the current line number for #io. # # #line counts the number of times #gets is called. @@ -250,6 +249,7 @@ def inspect class RelineInputMethod < InputMethod HISTORY = Reline::HISTORY + include HistorySavingAbility # Creates a new input method object using Reline def initialize IRB.__send__(:set_encoding, Reline.encoding_system_needs.name, override: false) @@ -451,10 +451,6 @@ def inspect str += " and #{inputrc_path}" if File.exist?(inputrc_path) str end - - def support_history_saving? - true - end end class ReidlineInputMethod < RelineInputMethod diff --git a/test/irb/test_history.rb b/test/irb/test_history.rb index f0dfa03c6..39f9e8275 100644 --- a/test/irb/test_history.rb +++ b/test/irb/test_history.rb @@ -1,9 +1,12 @@ # frozen_string_literal: false require 'irb' require 'readline' +require "tempfile" require_relative "helper" +return if RUBY_PLATFORM.match?(/solaris|mswin|mingw/i) + module TestIRB class HistoryTest < TestCase def setup @@ -205,4 +208,87 @@ def with_temp_stdio end end end -end if not RUBY_PLATFORM.match?(/solaris|mswin|mingw/i) + + class NestedIRBHistoryTest < IntegrationTestCase + def test_history_saving_with_nested_sessions + write_history "" + + write_ruby <<~'RUBY' + def foo + binding.irb + end + + binding.irb + RUBY + + run_ruby_file do + type "'outer session'" + type "foo" + type "'inner session'" + type "exit" + type "'outer session again'" + type "exit" + end + + assert_equal <<~HISTORY, @history_file.open.read + 'outer session' + foo + 'inner session' + exit + 'outer session again' + exit + HISTORY + end + + def test_history_saving_with_nested_sessions_and_prior_history + write_history <<~HISTORY + old_history_1 + old_history_2 + old_history_3 + HISTORY + + write_ruby <<~'RUBY' + def foo + binding.irb + end + + binding.irb + RUBY + + run_ruby_file do + type "'outer session'" + type "foo" + type "'inner session'" + type "exit" + type "'outer session again'" + type "exit" + end + + assert_equal <<~HISTORY, @history_file.open.read + old_history_1 + old_history_2 + old_history_3 + 'outer session' + foo + 'inner session' + exit + 'outer session again' + exit + HISTORY + end + + private + + def write_history(history) + @history_file = Tempfile.new('irb_history') + @history_file.write(history) + @history_file.close + @irbrc = Tempfile.new('irbrc') + @irbrc.write <<~RUBY + IRB.conf[:HISTORY_FILE] = "#{@history_file.path}" + RUBY + @irbrc.close + @envs['IRBRC'] = @irbrc.path + end + end +end