Handle Concurrent Sessions and Saving Readline::HISTORY (#651)

* handle concurrent sessions and saving Readline::HISTORY, fixes #510

* separate tests

* don't mutate the HISTORY object on the class

* avoid repeated .to_i calls

* remove intermediary history array

* work with array, fix test comment

---------

Co-authored-by: Stan Lo <stan001212@gmail.com>

Author: chadrschroeder

lib/irb/history.rb

@@ -10,6 +10,7 @@ def reset_history_counter
 
     def load_history
       history = self.class::HISTORY
+
       if history_file = IRB.conf[:HISTORY_FILE]
         history_file = File.expand_path(history_file)
       end
@@ -32,7 +33,8 @@ def load_history
     end
 
     def save_history
-      history = self.class::HISTORY
+      history = self.class::HISTORY.to_a
+
       if num = IRB.conf[:SAVE_HISTORY] and (num = num.to_i) != 0
         if history_file = IRB.conf[:HISTORY_FILE]
           history_file = File.expand_path(history_file)

test/irb/test_history.rb

@@ -17,8 +17,16 @@ def teardown
       IRB.conf[:RC_NAME_GENERATOR] = nil
     end
 
-    class TestInputMethodWithHistory < TestInputMethod
-      HISTORY = Array.new
+    class TestInputMethodWithRelineHistory < TestInputMethod
+      # When IRB.conf[:USE_MULTILINE] is true, IRB::RelineInputMethod uses Reline::History
+      HISTORY = Reline::History.new(Reline.core.config)
+
+      include IRB::HistorySavingAbility
+    end
+
+    class TestInputMethodWithReadlineHistory < TestInputMethod
+      # When IRB.conf[:USE_MULTILINE] is false, IRB::ReadlineInputMethod uses Readline::HISTORY
+      HISTORY = Readline::HISTORY
 
       include IRB::HistorySavingAbility
     end
@@ -102,35 +110,16 @@ def test_history_save_minus_as_infinity
       INPUT
     end
 
-    def test_history_concurrent_use
+    def test_history_concurrent_use_reline
       omit "Skip Editline" if /EditLine/n.match(Readline::VERSION)
       IRB.conf[:SAVE_HISTORY] = 1
-      assert_history(<<~EXPECTED_HISTORY, <<~INITIAL_HISTORY, <<~INPUT) do |history_file|
-        exit
-        5
-        exit
-      EXPECTED_HISTORY
-        1
-        2
-        3
-        4
-      INITIAL_HISTORY
-        5
-        exit
-      INPUT
-        assert_history(<<~EXPECTED_HISTORY2, <<~INITIAL_HISTORY2, <<~INPUT2)
-        exit
-      EXPECTED_HISTORY2
-        1
-        2
-        3
-        4
-      INITIAL_HISTORY2
-        5
-        exit
-      INPUT2
-        File.utime(File.atime(history_file), File.mtime(history_file) + 2, history_file)
-      end
+      history_concurrent_use_for_input_method(TestInputMethodWithRelineHistory)
+    end
+
+    def test_history_concurrent_use_readline
+      omit "Skip Editline" if /EditLine/n.match(Readline::VERSION)
+      IRB.conf[:SAVE_HISTORY] = 1
+      history_concurrent_use_for_input_method(TestInputMethodWithReadlineHistory)
     end
 
     def test_history_concurrent_use_not_present
@@ -141,10 +130,11 @@ def test_history_concurrent_use_not_present
       IRB.conf[:SAVE_HISTORY] = 1
       Dir.mktmpdir("test_irb_history_") do |tmpdir|
         ENV["HOME"] = tmpdir
-        io = TestInputMethodWithHistory.new
+        io = TestInputMethodWithRelineHistory.new
         io.class::HISTORY.clear
         io.load_history
-        io.class::HISTORY.concat(%w"line1 line2")
+        io.class::HISTORY << 'line1'
+        io.class::HISTORY << 'line2'
 
         history_file = IRB.rc_file("_history")
         assert_not_send [File, :file?, history_file]
@@ -160,7 +150,36 @@ def test_history_concurrent_use_not_present
 
     private
 
-    def assert_history(expected_history, initial_irb_history, input)
+    def history_concurrent_use_for_input_method(input_method)
+      assert_history(<<~EXPECTED_HISTORY, <<~INITIAL_HISTORY, <<~INPUT, input_method) do |history_file|
+        exit
+        5
+        exit
+      EXPECTED_HISTORY
+        1
+        2
+        3
+        4
+      INITIAL_HISTORY
+        5
+        exit
+      INPUT
+        assert_history(<<~EXPECTED_HISTORY2, <<~INITIAL_HISTORY2, <<~INPUT2, input_method)
+        exit
+      EXPECTED_HISTORY2
+        1
+        2
+        3
+        4
+      INITIAL_HISTORY2
+        5
+        exit
+      INPUT2
+        File.utime(File.atime(history_file), File.mtime(history_file) + 2, history_file)
+      end
+    end
+
+    def assert_history(expected_history, initial_irb_history, input, input_method = TestInputMethodWithRelineHistory)
       backup_verbose, $VERBOSE = $VERBOSE, nil
       backup_home = ENV["HOME"]
       backup_xdg_config_home = ENV.delete("XDG_CONFIG_HOME")
@@ -172,15 +191,17 @@ def assert_history(expected_history, initial_irb_history, input)
           f.write(initial_irb_history)
         end
 
-        io = TestInputMethodWithHistory.new
+        io = input_method.new
         io.class::HISTORY.clear
         io.load_history
         if block_given?
-          history = io.class::HISTORY.dup
+          previous_history = []
+          io.class::HISTORY.each { |line| previous_history << line }
           yield IRB.rc_file("_history")
-          io.class::HISTORY.replace(history)
+          io.class::HISTORY.clear
+          previous_history.each { |line| io.class::HISTORY << line }
         end
-        io.class::HISTORY.concat(input.split)
+        input.split.each { |line| io.class::HISTORY << line }
         io.save_history
 
         io.load_history