From bda5b099375d91274a5314aad2608f8e5b37e891 Mon Sep 17 00:00:00 2001 From: tomoya ishida Date: Sun, 17 Mar 2024 00:19:59 +0900 Subject: [PATCH] [ruby/irb] Fix irb_history saved to current directory (https://github.com/ruby/irb/pull/901) * Always save irb_history in HOME or XDG_CONFIG_HOME Also split irbrc search logic from irb_history search logic as a refactor * Remove IRB.conf[:RC_NAME_GENERATOR] because it's not configurable This conf is used to specify which irbrc to load. Need to configure before irbrc is loaded, so it's actually not configurable. This conf is also used for history file search, but it is configurable by conf[:HISTORY_FILE]. * remove rc_file_test because it is tested with rc_files, remove useless test setup * Make internal irbrc searching method private https://github.com/ruby/irb/commit/11d03a6ff7 --- doc/irb/indexes.md | 3 - lib/irb/command/irb_info.rb | 2 +- lib/irb/history.rb | 9 +- lib/irb/init.rb | 114 ++++++++------- lib/irb/lc/error.rb | 5 - lib/irb/lc/ja/error.rb | 5 - test/irb/test_command.rb | 1 + test/irb/test_history.rb | 18 ++- test/irb/test_init.rb | 173 +++++++---------------- test/irb/yamatanooroti/test_rendering.rb | 6 + 10 files changed, 148 insertions(+), 188 deletions(-) diff --git a/doc/irb/indexes.md b/doc/irb/indexes.md index 9659db8c0b..24a67b9698 100644 --- a/doc/irb/indexes.md +++ b/doc/irb/indexes.md @@ -165,9 +165,6 @@ for each entry that is pre-defined, the initial value is given: - `:RC`: Whether a {configuration file}[rdoc-ref:IRB@Configuration+File] was found and interpreted; initial value: `true` if a configuration file was found, `false` otherwise. -- `:RC_NAME_GENERATOR`: \Proc to generate paths of potential - {configuration files}[rdoc-ref:IRB@Configuration+File]; - initial value: `=> #`. - `:SAVE_HISTORY`: Number of commands to save in {input command history}[rdoc-ref:IRB@Input+Command+History]; initial value: `1000`. diff --git a/lib/irb/command/irb_info.rb b/lib/irb/command/irb_info.rb index 31e6d77d25..cc93fdcbd5 100644 --- a/lib/irb/command/irb_info.rb +++ b/lib/irb/command/irb_info.rb @@ -13,7 +13,7 @@ module IRB str += "IRB version: #{IRB.version}\n" str += "InputMethod: #{IRB.CurrentContext.io.inspect}\n" str += "Completion: #{IRB.CurrentContext.io.respond_to?(:completion_info) ? IRB.CurrentContext.io.completion_info : 'off'}\n" - rc_files = IRB.rc_files.select { |rc| File.exist?(rc) } + rc_files = IRB.irbrc_files str += ".irbrc paths: #{rc_files.join(", ")}\n" if rc_files.any? str += "RUBY_PLATFORM: #{RUBY_PLATFORM}\n" str += "LANG env: #{ENV["LANG"]}\n" if ENV["LANG"] && !ENV["LANG"].empty? diff --git a/lib/irb/history.rb b/lib/irb/history.rb index 2489f7403f..685354b2d8 100644 --- a/lib/irb/history.rb +++ b/lib/irb/history.rb @@ -16,8 +16,8 @@ module IRB if history_file = IRB.conf[:HISTORY_FILE] history_file = File.expand_path(history_file) end - history_file = IRB.rc_files("_history").first unless history_file - if File.exist?(history_file) + history_file = IRB.rc_file("_history") unless history_file + if history_file && File.exist?(history_file) File.open(history_file, "r:#{IRB.conf[:LC_MESSAGES].encoding}") do |f| f.each { |l| l = l.chomp @@ -41,7 +41,10 @@ module IRB if history_file = IRB.conf[:HISTORY_FILE] history_file = File.expand_path(history_file) end - history_file = IRB.rc_files("_history").first unless history_file + history_file = IRB.rc_file("_history") unless history_file + + # When HOME and XDG_CONFIG_HOME are not available, history_file might be nil + return unless history_file # Change the permission of a file that already exists[BUG #7694] begin diff --git a/lib/irb/init.rb b/lib/irb/init.rb index 5813ff7ae0..355047519c 100644 --- a/lib/irb/init.rb +++ b/lib/irb/init.rb @@ -395,10 +395,8 @@ module IRB # :nodoc: # Run the config file def IRB.run_config if @CONF[:RC] - rc_files.each do |rc| - # Because rc_file always returns `HOME/.irbrc` even if no rc file is present, we can't warn users about missing rc files. - # Otherwise, it'd be very noisy. - load rc if File.exist?(rc) + irbrc_files.each do |rc| + load rc rescue StandardError, ScriptError => e warn "Error loading RC file '#{rc}':\n#{e.full_message(highlight: false)}" end @@ -406,53 +404,27 @@ module IRB # :nodoc: end IRBRC_EXT = "rc" - def IRB.rc_file(ext = IRBRC_EXT) - warn "rc_file is deprecated, please use rc_files instead." - rc_files(ext).first + + def IRB.rc_file(ext) + prepare_irbrc_name_generators + + # When irbrc exist in default location + if (rcgen = @existing_rc_name_generators.first) + return rcgen.call(ext) + end + + # When irbrc does not exist in default location + rc_file_generators do |rcgen| + return rcgen.call(ext) + end + + # When HOME and XDG_CONFIG_HOME are not available + nil end - def IRB.rc_files(ext = IRBRC_EXT) - if !@CONF[:RC_NAME_GENERATOR] - @CONF[:RC_NAME_GENERATOR] ||= [] - existing_rc_file_generators = [] - - rc_file_generators do |rcgen| - @CONF[:RC_NAME_GENERATOR] << rcgen - existing_rc_file_generators << rcgen if File.exist?(rcgen.call(ext)) - end - - if existing_rc_file_generators.any? - @CONF[:RC_NAME_GENERATOR] = existing_rc_file_generators - end - end - - @CONF[:RC_NAME_GENERATOR].map do |rc| - rc_file = rc.call(ext) - fail IllegalRCNameGenerator unless rc_file.is_a?(String) - rc_file - end - end - - # enumerate possible rc-file base name generators - def IRB.rc_file_generators - if irbrc = ENV["IRBRC"] - yield proc{|rc| rc == "rc" ? irbrc : irbrc+rc} - end - if xdg_config_home = ENV["XDG_CONFIG_HOME"] - irb_home = File.join(xdg_config_home, "irb") - if File.directory?(irb_home) - yield proc{|rc| irb_home + "/irb#{rc}"} - end - end - if home = ENV["HOME"] - yield proc{|rc| home+"/.irb#{rc}"} - yield proc{|rc| home+"/.config/irb/irb#{rc}"} - end - current_dir = Dir.pwd - yield proc{|rc| current_dir+"/.irb#{rc}"} - yield proc{|rc| current_dir+"/irb#{rc.sub(/\A_?/, '.')}"} - yield proc{|rc| current_dir+"/_irb#{rc}"} - yield proc{|rc| current_dir+"/$irb#{rc}"} + def IRB.irbrc_files + prepare_irbrc_name_generators + @irbrc_files end # loading modules @@ -468,6 +440,50 @@ module IRB # :nodoc: class << IRB private + + def prepare_irbrc_name_generators + return if @existing_rc_name_generators + + @existing_rc_name_generators = [] + @irbrc_files = [] + rc_file_generators do |rcgen| + irbrc = rcgen.call(IRBRC_EXT) + if File.exist?(irbrc) + @irbrc_files << irbrc + @existing_rc_name_generators << rcgen + end + end + generate_current_dir_irbrc_files.each do |irbrc| + @irbrc_files << irbrc if File.exist?(irbrc) + end + @irbrc_files.uniq! + end + + # enumerate possible rc-file base name generators + def rc_file_generators + if irbrc = ENV["IRBRC"] + yield proc{|rc| rc == "rc" ? irbrc : irbrc+rc} + end + if xdg_config_home = ENV["XDG_CONFIG_HOME"] + irb_home = File.join(xdg_config_home, "irb") + if File.directory?(irb_home) + yield proc{|rc| irb_home + "/irb#{rc}"} + end + end + if home = ENV["HOME"] + yield proc{|rc| home+"/.irb#{rc}"} + if xdg_config_home.nil? || xdg_config_home.empty? + yield proc{|rc| home+"/.config/irb/irb#{rc}"} + end + end + end + + # possible irbrc files in current directory + def generate_current_dir_irbrc_files + current_dir = Dir.pwd + %w[.irbrc irbrc _irbrc $irbrc].map { |file| "#{current_dir}/#{file}" } + end + def set_encoding(extern, intern = nil, override: true) verbose, $VERBOSE = $VERBOSE, nil Encoding.default_external = extern unless extern.nil? || extern.empty? diff --git a/lib/irb/lc/error.rb b/lib/irb/lc/error.rb index 9041ec4d6c..ee0f047822 100644 --- a/lib/irb/lc/error.rb +++ b/lib/irb/lc/error.rb @@ -47,11 +47,6 @@ module IRB super("Undefined prompt mode(#{val}).") end end - class IllegalRCGenerator < StandardError - def initialize - super("Define illegal RC_NAME_GENERATOR.") - end - end # :startdoc: end diff --git a/lib/irb/lc/ja/error.rb b/lib/irb/lc/ja/error.rb index f7f2b13c45..9e2e5b8870 100644 --- a/lib/irb/lc/ja/error.rb +++ b/lib/irb/lc/ja/error.rb @@ -47,11 +47,6 @@ module IRB super("プロンプトモード(#{val})は定義されていません.") end end - class IllegalRCGenerator < StandardError - def initialize - super("RC_NAME_GENERATORが正しく定義されていません.") - end - end # :startdoc: end diff --git a/test/irb/test_command.rb b/test/irb/test_command.rb index 8e074e97f9..a8af0762a8 100644 --- a/test/irb/test_command.rb +++ b/test/irb/test_command.rb @@ -20,6 +20,7 @@ module TestIRB @xdg_config_home_backup = ENV.delete("XDG_CONFIG_HOME") save_encodings IRB.instance_variable_get(:@CONF).clear + IRB.instance_variable_set(:@existing_rc_name_generators, nil) @is_win = (RbConfig::CONFIG['host_os'] =~ /mswin|msys|mingw|cygwin|bccwin|wince|emc/) end diff --git a/test/irb/test_history.rb b/test/irb/test_history.rb index ea220a2dd3..63be35fdaa 100644 --- a/test/irb/test_history.rb +++ b/test/irb/test_history.rb @@ -17,11 +17,11 @@ module TestIRB @backup_irbrc = ENV.delete("IRBRC") @backup_default_external = Encoding.default_external ENV["HOME"] = @tmpdir - IRB.conf[:RC_NAME_GENERATOR] = nil + IRB.instance_variable_set(:@existing_rc_name_generators, nil) end def teardown - IRB.conf[:RC_NAME_GENERATOR] = nil + IRB.instance_variable_set(:@existing_rc_name_generators, nil) ENV["HOME"] = @backup_home ENV["XDG_CONFIG_HOME"] = @backup_xdg_config_home ENV["IRBRC"] = @backup_irbrc @@ -144,7 +144,7 @@ module TestIRB io.class::HISTORY << 'line1' io.class::HISTORY << 'line2' - history_file = IRB.rc_files("_history").first + history_file = IRB.rc_file("_history") assert_not_send [File, :file?, history_file] File.write(history_file, "line0\n") io.save_history @@ -183,6 +183,16 @@ module TestIRB IRB.conf[:HISTORY_FILE] = backup_history_file end + def test_no_home_no_history_file_does_not_raise_history_save + ENV['HOME'] = nil + io = TestInputMethodWithRelineHistory.new + assert_nil(IRB.rc_file('_history')) + assert_nothing_raised do + io.load_history + io.save_history + end + end + private def history_concurrent_use_for_input_method(input_method) @@ -217,7 +227,7 @@ module TestIRB def assert_history(expected_history, initial_irb_history, input, input_method = TestInputMethodWithRelineHistory, locale: IRB::Locale.new) IRB.conf[:LC_MESSAGES] = locale actual_history = nil - history_file = IRB.rc_files("_history").first + history_file = IRB.rc_file("_history") ENV["HOME"] = @tmpdir File.open(history_file, "w") do |f| f.write(initial_irb_history) diff --git a/test/irb/test_init.rb b/test/irb/test_init.rb index fd1e06e39e..f11d7398c8 100644 --- a/test/irb/test_init.rb +++ b/test/irb/test_init.rb @@ -14,10 +14,15 @@ module TestIRB ENV["HOME"] = @tmpdir = File.realpath(Dir.mktmpdir("test_irb_init_#{$$}")) end + def reset_rc_name_generators + IRB.instance_variable_set(:@existing_rc_name_generators, nil) + end + def teardown ENV.update(@backup_env) FileUtils.rm_rf(@tmpdir) IRB.conf.delete(:SCRIPT) + reset_rc_name_generators end def test_setup_with_argv_preserves_global_argv @@ -34,133 +39,65 @@ module TestIRB assert_equal orig, $0 end - def test_rc_file - verbose, $VERBOSE = $VERBOSE, nil - tmpdir = @tmpdir - Dir.chdir(tmpdir) do - ENV["XDG_CONFIG_HOME"] = "#{tmpdir}/xdg" - IRB.conf[:RC_NAME_GENERATOR] = nil - assert_equal(tmpdir+"/.irbrc", IRB.rc_file) - assert_equal(tmpdir+"/.irb_history", IRB.rc_file("_history")) - assert_file.not_exist?(tmpdir+"/xdg") - IRB.conf[:RC_NAME_GENERATOR] = nil - FileUtils.touch(tmpdir+"/.irbrc") - assert_equal(tmpdir+"/.irbrc", IRB.rc_file) - assert_equal(tmpdir+"/.irb_history", IRB.rc_file("_history")) - assert_file.not_exist?(tmpdir+"/xdg") - end - ensure - $VERBOSE = verbose - end - - def test_rc_file_in_subdir - verbose, $VERBOSE = $VERBOSE, nil - tmpdir = @tmpdir - Dir.chdir(tmpdir) do - FileUtils.mkdir_p("#{tmpdir}/mydir") - Dir.chdir("#{tmpdir}/mydir") do - IRB.conf[:RC_NAME_GENERATOR] = nil - assert_equal(tmpdir+"/.irbrc", IRB.rc_file) - assert_equal(tmpdir+"/.irb_history", IRB.rc_file("_history")) - IRB.conf[:RC_NAME_GENERATOR] = nil - FileUtils.touch(tmpdir+"/.irbrc") - assert_equal(tmpdir+"/.irbrc", IRB.rc_file) - assert_equal(tmpdir+"/.irb_history", IRB.rc_file("_history")) - end - end - ensure - $VERBOSE = verbose - end - def test_rc_files tmpdir = @tmpdir Dir.chdir(tmpdir) do - ENV["XDG_CONFIG_HOME"] = "#{tmpdir}/xdg" - IRB.conf[:RC_NAME_GENERATOR] = nil - assert_includes IRB.rc_files, tmpdir+"/.irbrc" - assert_includes IRB.rc_files("_history"), tmpdir+"/.irb_history" - assert_file.not_exist?(tmpdir+"/xdg") - IRB.conf[:RC_NAME_GENERATOR] = nil - FileUtils.touch(tmpdir+"/.irbrc") - assert_includes IRB.rc_files, tmpdir+"/.irbrc" - assert_includes IRB.rc_files("_history"), tmpdir+"/.irb_history" - assert_file.not_exist?(tmpdir+"/xdg") - end - end - - def test_rc_files_in_subdir - tmpdir = @tmpdir - Dir.chdir(tmpdir) do - FileUtils.mkdir_p("#{tmpdir}/mydir") - Dir.chdir("#{tmpdir}/mydir") do - IRB.conf[:RC_NAME_GENERATOR] = nil - assert_includes IRB.rc_files, tmpdir+"/.irbrc" - assert_includes IRB.rc_files("_history"), tmpdir+"/.irb_history" - IRB.conf[:RC_NAME_GENERATOR] = nil - FileUtils.touch(tmpdir+"/.irbrc") - assert_includes IRB.rc_files, tmpdir+"/.irbrc" - assert_includes IRB.rc_files("_history"), tmpdir+"/.irb_history" + home = ENV['HOME'] = "#{tmpdir}/home" + xdg_config_home = ENV['XDG_CONFIG_HOME'] = "#{tmpdir}/xdg" + reset_rc_name_generators + assert_empty(IRB.irbrc_files) + assert_equal("#{home}/.irb_history", IRB.rc_file('_history')) + FileUtils.mkdir_p(home) + FileUtils.mkdir_p("#{xdg_config_home}/irb") + FileUtils.mkdir_p("#{home}/.config/irb") + reset_rc_name_generators + assert_empty(IRB.irbrc_files) + assert_equal("#{xdg_config_home}/irb/irb_history", IRB.rc_file('_history')) + home_irbrc = "#{home}/.irbrc" + config_irbrc = "#{home}/.config/irb/irbrc" + xdg_config_irbrc = "#{xdg_config_home}/irb/irbrc" + [home_irbrc, config_irbrc, xdg_config_irbrc].each do |file| + FileUtils.touch(file) end + current_dir_irbrcs = %w[.irbrc irbrc _irbrc $irbrc].map { |file| "#{tmpdir}/#{file}" } + current_dir_irbrcs.each { |file| FileUtils.touch(file) } + reset_rc_name_generators + assert_equal([xdg_config_irbrc, home_irbrc, *current_dir_irbrcs], IRB.irbrc_files) + assert_equal(xdg_config_irbrc.sub(/rc$/, '_history'), IRB.rc_file('_history')) + ENV['XDG_CONFIG_HOME'] = nil + reset_rc_name_generators + assert_equal([home_irbrc, config_irbrc, *current_dir_irbrcs], IRB.irbrc_files) + assert_equal(home_irbrc.sub(/rc$/, '_history'), IRB.rc_file('_history')) + ENV['XDG_CONFIG_HOME'] = '' + reset_rc_name_generators + assert_equal([home_irbrc, config_irbrc] + current_dir_irbrcs, IRB.irbrc_files) + assert_equal(home_irbrc.sub(/rc$/, '_history'), IRB.rc_file('_history')) + ENV['XDG_CONFIG_HOME'] = xdg_config_home + ENV['IRBRC'] = "#{tmpdir}/.irbrc" + reset_rc_name_generators + assert_equal([ENV['IRBRC'], xdg_config_irbrc, home_irbrc] + (current_dir_irbrcs - [ENV['IRBRC']]), IRB.irbrc_files) + assert_equal(ENV['IRBRC'] + '_history', IRB.rc_file('_history')) + ENV['IRBRC'] = ENV['HOME'] = ENV['XDG_CONFIG_HOME'] = nil + reset_rc_name_generators + assert_equal(current_dir_irbrcs, IRB.irbrc_files) + assert_nil(IRB.rc_file('_history')) end end - def test_rc_files_has_file_from_xdg_env + def test_duplicated_rc_files tmpdir = @tmpdir - ENV["XDG_CONFIG_HOME"] = "#{tmpdir}/xdg" - xdg_config = ENV["XDG_CONFIG_HOME"]+"/irb/irbrc" - - FileUtils.mkdir_p(xdg_config) - Dir.chdir(tmpdir) do - IRB.conf[:RC_NAME_GENERATOR] = nil - assert_includes IRB.rc_files, xdg_config - end - ensure - ENV["XDG_CONFIG_HOME"] = nil - end - - def test_rc_files_has_file_from_irbrc_env - tmpdir = @tmpdir - ENV["IRBRC"] = "#{tmpdir}/irb" - - FileUtils.mkdir_p(ENV["IRBRC"]) - - Dir.chdir(tmpdir) do - IRB.conf[:RC_NAME_GENERATOR] = nil - assert_includes IRB.rc_files, ENV["IRBRC"] - end - ensure - ENV["IRBRC"] = nil - end - - def test_rc_files_has_file_from_home_env - tmpdir = @tmpdir - ENV["HOME"] = "#{tmpdir}/home" - - FileUtils.mkdir_p(ENV["HOME"]) - - Dir.chdir(tmpdir) do - IRB.conf[:RC_NAME_GENERATOR] = nil - assert_includes IRB.rc_files, ENV["HOME"]+"/.irbrc" - assert_includes IRB.rc_files, ENV["HOME"]+"/.config/irb/irbrc" - end - ensure - ENV["HOME"] = nil - end - - def test_rc_files_contains_non_env_files - tmpdir = @tmpdir - FileUtils.mkdir_p("#{tmpdir}/.irbrc") - FileUtils.mkdir_p("#{tmpdir}/_irbrc") - FileUtils.mkdir_p("#{tmpdir}/irb.rc") - FileUtils.mkdir_p("#{tmpdir}/$irbrc") - - Dir.chdir(tmpdir) do - IRB.conf[:RC_NAME_GENERATOR] = nil - assert_includes IRB.rc_files, tmpdir+"/.irbrc" - assert_includes IRB.rc_files, tmpdir+"/_irbrc" - assert_includes IRB.rc_files, tmpdir+"/irb.rc" - assert_includes IRB.rc_files, tmpdir+"/$irbrc" + ENV['XDG_CONFIG_HOME'] = "#{ENV['HOME']}/.config" + FileUtils.mkdir_p("#{ENV['XDG_CONFIG_HOME']}/irb") + env_irbrc = ENV['IRBRC'] = "#{tmpdir}/_irbrc" + xdg_config_irbrc = "#{ENV['XDG_CONFIG_HOME']}/irb/irbrc" + home_irbrc = "#{ENV['HOME']}/.irbrc" + current_dir_irbrc = "#{tmpdir}/irbrc" + [env_irbrc, xdg_config_irbrc, home_irbrc, current_dir_irbrc].each do |file| + FileUtils.touch(file) + end + reset_rc_name_generators + assert_equal([env_irbrc, xdg_config_irbrc, home_irbrc, current_dir_irbrc], IRB.irbrc_files) end end diff --git a/test/irb/yamatanooroti/test_rendering.rb b/test/irb/yamatanooroti/test_rendering.rb index a0477ee482..511df58ea6 100644 --- a/test/irb/yamatanooroti/test_rendering.rb +++ b/test/irb/yamatanooroti/test_rendering.rb @@ -11,6 +11,8 @@ end class IRB::RenderingTest < Yamatanooroti::TestCase def setup @original_term = ENV['TERM'] + @home_backup = ENV['HOME'] + @xdg_config_home_backup = ENV['XDG_CONFIG_HOME'] ENV['TERM'] = "xterm-256color" @pwd = Dir.pwd suffix = '%010d' % Random.rand(0..65535) @@ -24,12 +26,16 @@ class IRB::RenderingTest < Yamatanooroti::TestCase @irbrc_backup = ENV['IRBRC'] @irbrc_file = ENV['IRBRC'] = File.join(@tmpdir, 'temporaty_irbrc') File.unlink(@irbrc_file) if File.exist?(@irbrc_file) + ENV['HOME'] = File.join(@tmpdir, 'home') + ENV['XDG_CONFIG_HOME'] = File.join(@tmpdir, 'xdg_config_home') end def teardown FileUtils.rm_rf(@tmpdir) ENV['IRBRC'] = @irbrc_backup ENV['TERM'] = @original_term + ENV['HOME'] = @home_backup + ENV['XDG_CONFIG_HOME'] = @xdg_config_home_backup end def test_launch