Skip to content

Commit

Permalink
Fix nested IRB sessions' history saving
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
st0012 committed Aug 4, 2023
1 parent bbd2044 commit a711419
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 23 deletions.
8 changes: 8 additions & 0 deletions lib/irb.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
10 changes: 0 additions & 10 deletions lib/irb/context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]
Expand All @@ -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
Expand Down
6 changes: 2 additions & 4 deletions lib/irb/history.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down
12 changes: 4 additions & 8 deletions lib/irb/input-method.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#

require_relative 'completion'
require_relative "history"
require 'io/console'
require 'reline'

Expand Down Expand Up @@ -180,6 +181,8 @@ def self.initialize_readline
include ::Readline
end

include HistorySavingAbility

# Creates a new input method object using Readline
def initialize
self.class.initialize_readline
Expand Down Expand Up @@ -233,10 +236,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.
Expand Down Expand Up @@ -264,6 +263,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)
Expand Down Expand Up @@ -466,10 +466,6 @@ def inspect
str += " and #{inputrc_path}" if File.exist?(inputrc_path)
str
end

def support_history_saving?
true
end
end

class ReidlineInputMethod < RelineInputMethod
Expand Down
58 changes: 57 additions & 1 deletion test/irb/test_history.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -205,4 +208,57 @@ 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_debug
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('rdbg_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

0 comments on commit a711419

Please sign in to comment.