From a3ec5eb969b40a47cf924dbf444a177b691217bd Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sat, 24 Feb 2024 08:01:29 +1300 Subject: [PATCH 1/4] feat: add option to have `gsub_file` error if file contents don't change --- lib/thor/actions/file_manipulation.rb | 10 ++++++++-- spec/actions/file_manipulation_spec.rb | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/lib/thor/actions/file_manipulation.rb b/lib/thor/actions/file_manipulation.rb index 8eec045e..e63f6006 100644 --- a/lib/thor/actions/file_manipulation.rb +++ b/lib/thor/actions/file_manipulation.rb @@ -248,7 +248,8 @@ def inject_into_module(path, module_name, *args, &block) # path:: path of the file to be changed # flag:: the regexp or string to be replaced # replacement:: the replacement, can be also given as a block - # config:: give :verbose => false to not log the status, and + # config:: give :verbose => false to not log the status, + # :error_on_no_change => true to raise an error if the file does not change, and # :force => true, to force the replacement regardless of runner behavior. # # ==== Example @@ -269,7 +270,12 @@ def gsub_file(path, flag, *args, &block) unless options[:pretend] content = File.binread(path) - content.gsub!(flag, *args, &block) + success = content.gsub!(flag, *args, &block) + + if success.nil? && config.fetch(:error_on_no_change, false) + raise Thor::Error, "The content of #{path} did not change" + end + File.open(path, "wb") { |file| file.write(content) } end end diff --git a/spec/actions/file_manipulation_spec.rb b/spec/actions/file_manipulation_spec.rb index 72919e09..ab449c0f 100644 --- a/spec/actions/file_manipulation_spec.rb +++ b/spec/actions/file_manipulation_spec.rb @@ -318,6 +318,26 @@ def file it "does not log status if required" do expect(action(:gsub_file, file, "__", verbose: false) { |match| match * 2 }).to be_empty end + + it "does not care if the file contents did not change" do + action :gsub_file, "doc/README", "___start___", "START" + expect(File.binread(file)).to eq("__start__\nREADME\n__end__\n") + end + + context "with error_on_no_change" do + it "replaces the content in the file" do + action :gsub_file, "doc/README", "__start__", "START", error_on_no_change: true + expect(File.binread(file)).to eq("START\nREADME\n__end__\n") + end + + it "raises if the file contents did not change" do + expect do + action :gsub_file, "doc/README", "___start___", "START", error_on_no_change: true + end.to raise_error(Thor::Error, "The content of #{destination_root}/doc/README did not change") + + expect(File.binread(file)).to eq("__start__\nREADME\n__end__\n") + end + end end context "with revoke behavior" do From 489b4f165e8b15a78f15b5686507340be30bbaea Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sat, 24 Feb 2024 08:02:06 +1300 Subject: [PATCH 2/4] feat: add `gsub_file!` method --- lib/thor/actions/file_manipulation.rb | 26 +++++++ spec/actions/file_manipulation_spec.rb | 98 ++++++++++++++++++++++++++ 2 files changed, 124 insertions(+) diff --git a/lib/thor/actions/file_manipulation.rb b/lib/thor/actions/file_manipulation.rb index e63f6006..5edafe42 100644 --- a/lib/thor/actions/file_manipulation.rb +++ b/lib/thor/actions/file_manipulation.rb @@ -242,6 +242,32 @@ def inject_into_module(path, module_name, *args, &block) insert_into_file(path, *(args << config), &block) end + # Run a regular expression replacement on a file, raising an error if the + # contents of the file are not changed. + # + # ==== Parameters + # path:: path of the file to be changed + # flag:: the regexp or string to be replaced + # replacement:: the replacement, can be also given as a block + # config:: give :verbose => false to not log the status, and + # :force => true, to force the replacement regardless of runner behavior. + # + # ==== Example + # + # gsub_file 'app/controllers/application_controller.rb', /#\s*(filter_parameter_logging :password)/, '\1' + # + # gsub_file 'README', /rake/, :green do |match| + # match << " no more. Use thor!" + # end + # + def gsub_file!(path, flag, *args, &block) + config = args.last.is_a?(Hash) ? args.pop : {} + + config[:error_on_no_change] = true + + gsub_file(path, flag, *args, config, &block) + end + # Run a regular expression replacement on a file. # # ==== Parameters diff --git a/spec/actions/file_manipulation_spec.rb b/spec/actions/file_manipulation_spec.rb index ab449c0f..b473345c 100644 --- a/spec/actions/file_manipulation_spec.rb +++ b/spec/actions/file_manipulation_spec.rb @@ -293,6 +293,104 @@ def file end end + describe "#gsub_file!" do + context "with invoke behavior" do + it "replaces the content in the file" do + action :gsub_file!, "doc/README", "__start__", "START" + expect(File.binread(file)).to eq("START\nREADME\n__end__\n") + end + + it "does not replace if pretending" do + runner(pretend: true) + action :gsub_file!, "doc/README", "__start__", "START" + expect(File.binread(file)).to eq("__start__\nREADME\n__end__\n") + end + + it "accepts a block" do + action(:gsub_file!, "doc/README", "__start__") { |match| match.gsub("__", "").upcase } + expect(File.binread(file)).to eq("START\nREADME\n__end__\n") + end + + it "logs status" do + expect(action(:gsub_file!, "doc/README", "__start__", "START")).to eq(" gsub doc/README\n") + end + + it "does not log status if required" do + expect(action(:gsub_file!, file, "__", verbose: false) { |match| match * 2 }).to be_empty + end + + it "cares if the file contents did not change" do + expect do + action :gsub_file!, "doc/README", "___start___", "START" + end.to raise_error(Thor::Error, "The content of #{destination_root}/doc/README did not change") + + expect(File.binread(file)).to eq("__start__\nREADME\n__end__\n") + end + end + + context "with revoke behavior" do + context "and no force option" do + it "does not replace the content in the file" do + runner({}, :revoke) + action :gsub_file!, "doc/README", "__start__", "START" + expect(File.binread(file)).to eq("__start__\nREADME\n__end__\n") + end + + it "does not replace if pretending" do + runner({pretend: true}, :revoke) + action :gsub_file!, "doc/README", "__start__", "START" + expect(File.binread(file)).to eq("__start__\nREADME\n__end__\n") + end + + it "does not replace the content in the file when given a block" do + runner({}, :revoke) + action(:gsub_file!, "doc/README", "__start__") { |match| match.gsub("__", "").upcase } + expect(File.binread(file)).to eq("__start__\nREADME\n__end__\n") + end + + it "does not log status" do + runner({}, :revoke) + expect(action(:gsub_file!, "doc/README", "__start__", "START")).to be_empty + end + + it "does not log status if required" do + runner({}, :revoke) + expect(action(:gsub_file!, file, "__", verbose: false) { |match| match * 2 }).to be_empty + end + end + + context "and force option" do + it "replaces the content in the file" do + runner({}, :revoke) + action :gsub_file!, "doc/README", "__start__", "START", force: true + expect(File.binread(file)).to eq("START\nREADME\n__end__\n") + end + + it "does not replace if pretending" do + runner({pretend: true}, :revoke) + action :gsub_file!, "doc/README", "__start__", "START", force: true + expect(File.binread(file)).to eq("__start__\nREADME\n__end__\n") + end + + it "replaces the content in the file when given a block" do + runner({}, :revoke) + action(:gsub_file!, "doc/README", "__start__", force: true) { |match| match.gsub("__", "").upcase } + expect(File.binread(file)).to eq("START\nREADME\n__end__\n") + end + + it "logs status" do + runner({}, :revoke) + expect(action(:gsub_file!, "doc/README", "__start__", "START", force: true)).to eq(" gsub doc/README\n") + end + + it "does not log status if required" do + runner({}, :revoke) + expect(action(:gsub_file!, file, "__", verbose: false, force: true) { |match| match * 2 }).to be_empty + end + end + end + end + describe "#gsub_file" do context "with invoke behavior" do it "replaces the content in the file" do From 620b6850b43b530dc11c69e524f71630c195c181 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Fri, 1 Mar 2024 08:04:54 +1300 Subject: [PATCH 3/4] feat: don't support erroring as an option on original `gsub_file` --- lib/thor/actions/file_manipulation.rb | 32 +++++++++++++++----------- spec/actions/file_manipulation_spec.rb | 15 ------------ 2 files changed, 18 insertions(+), 29 deletions(-) diff --git a/lib/thor/actions/file_manipulation.rb b/lib/thor/actions/file_manipulation.rb index 5edafe42..7a508292 100644 --- a/lib/thor/actions/file_manipulation.rb +++ b/lib/thor/actions/file_manipulation.rb @@ -263,9 +263,12 @@ def inject_into_module(path, module_name, *args, &block) def gsub_file!(path, flag, *args, &block) config = args.last.is_a?(Hash) ? args.pop : {} - config[:error_on_no_change] = true + return unless behavior == :invoke || config.fetch(:force, false) + + path = File.expand_path(path, destination_root) + say_status :gsub, relative_to_original_destination_root(path), config.fetch(:verbose, true) - gsub_file(path, flag, *args, config, &block) + actually_gsub_file(path, flag, args, true, &block) unless options[:pretend] end # Run a regular expression replacement on a file. @@ -274,8 +277,7 @@ def gsub_file!(path, flag, *args, &block) # path:: path of the file to be changed # flag:: the regexp or string to be replaced # replacement:: the replacement, can be also given as a block - # config:: give :verbose => false to not log the status, - # :error_on_no_change => true to raise an error if the file does not change, and + # config:: give :verbose => false to not log the status, and # :force => true, to force the replacement regardless of runner behavior. # # ==== Example @@ -294,16 +296,7 @@ def gsub_file(path, flag, *args, &block) path = File.expand_path(path, destination_root) say_status :gsub, relative_to_original_destination_root(path), config.fetch(:verbose, true) - unless options[:pretend] - content = File.binread(path) - success = content.gsub!(flag, *args, &block) - - if success.nil? && config.fetch(:error_on_no_change, false) - raise Thor::Error, "The content of #{path} did not change" - end - - File.open(path, "wb") { |file| file.write(content) } - end + actually_gsub_file(path, flag, args, false, &block) unless options[:pretend] end # Uncomment all lines matching a given regex. Preserves indentation before @@ -389,6 +382,17 @@ def with_output_buffer(buf = "".dup) #:nodoc: self.output_buffer = old_buffer end + def actually_gsub_file(path, flag, args, error_on_no_change, &block) + content = File.binread(path) + success = content.gsub!(flag, *args, &block) + + if success.nil? && error_on_no_change + raise Thor::Error, "The content of #{path} did not change" + end + + File.open(path, "wb") { |file| file.write(content) } + end + # Thor::Actions#capture depends on what kind of buffer is used in ERB. # Thus CapturableERB fixes ERB to use String buffer. class CapturableERB < ERB diff --git a/spec/actions/file_manipulation_spec.rb b/spec/actions/file_manipulation_spec.rb index b473345c..172dbcb3 100644 --- a/spec/actions/file_manipulation_spec.rb +++ b/spec/actions/file_manipulation_spec.rb @@ -421,21 +421,6 @@ def file action :gsub_file, "doc/README", "___start___", "START" expect(File.binread(file)).to eq("__start__\nREADME\n__end__\n") end - - context "with error_on_no_change" do - it "replaces the content in the file" do - action :gsub_file, "doc/README", "__start__", "START", error_on_no_change: true - expect(File.binread(file)).to eq("START\nREADME\n__end__\n") - end - - it "raises if the file contents did not change" do - expect do - action :gsub_file, "doc/README", "___start___", "START", error_on_no_change: true - end.to raise_error(Thor::Error, "The content of #{destination_root}/doc/README did not change") - - expect(File.binread(file)).to eq("__start__\nREADME\n__end__\n") - end - end end context "with revoke behavior" do From 54bc9decdcf4652041fc15fcf6afab334c7fa1fa Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 26 May 2024 10:02:41 +1200 Subject: [PATCH 4/4] fix: update docblock --- lib/thor/actions/file_manipulation.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/thor/actions/file_manipulation.rb b/lib/thor/actions/file_manipulation.rb index 7a508292..b16fbae0 100644 --- a/lib/thor/actions/file_manipulation.rb +++ b/lib/thor/actions/file_manipulation.rb @@ -254,9 +254,9 @@ def inject_into_module(path, module_name, *args, &block) # # ==== Example # - # gsub_file 'app/controllers/application_controller.rb', /#\s*(filter_parameter_logging :password)/, '\1' + # gsub_file! 'app/controllers/application_controller.rb', /#\s*(filter_parameter_logging :password)/, '\1' # - # gsub_file 'README', /rake/, :green do |match| + # gsub_file! 'README', /rake/, :green do |match| # match << " no more. Use thor!" # end #