From 87e2957f61d571ecf4a66181e458ef58ed966ef3 Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Thu, 1 Nov 2018 16:27:15 +0530 Subject: [PATCH] Warn on error and exit gracefully --- lib/jekyll-compose/file_creator.rb | 11 ++++++++--- lib/jekyll-compose/file_mover.rb | 22 ++++++++++++++++++++-- spec/draft_spec.rb | 12 ++++++------ spec/page_spec.rb | 12 ++++++------ spec/post_spec.rb | 12 ++++++------ spec/publish_spec.rb | 21 ++++++++++----------- spec/unpublish_spec.rb | 19 ++++++++----------- 7 files changed, 64 insertions(+), 45 deletions(-) diff --git a/lib/jekyll-compose/file_creator.rb b/lib/jekyll-compose/file_creator.rb index 17a7e0e..5867c06 100644 --- a/lib/jekyll-compose/file_creator.rb +++ b/lib/jekyll-compose/file_creator.rb @@ -11,7 +11,8 @@ def initialize(file_info, force = false, root = nil) end def create! - validate_should_write! + return unless create? + ensure_directory_exists write_file end @@ -24,8 +25,12 @@ def file_path private - def validate_should_write! - return Jekyll.logger.warn "A #{file.resource_type} already exists at #{file_path}".yellow if File.exist?(file_path) && !force + def create? + return true if force + return true unless File.exist?(file_path) + + Jekyll.logger.warn "A #{file.resource_type} already exists at #{file_path}" + false end def ensure_directory_exists diff --git a/lib/jekyll-compose/file_mover.rb b/lib/jekyll-compose/file_mover.rb index d658041..be7ef67 100644 --- a/lib/jekyll-compose/file_mover.rb +++ b/lib/jekyll-compose/file_mover.rb @@ -19,8 +19,8 @@ def resource_type_to end def move - validate_source - validate_should_write! + return unless valid_source? && valid_destination? + ensure_directory_exists move_file end @@ -45,6 +45,24 @@ def move_file private + def valid_source? + return true if File.exist?(from) + + invalidate_with "There was no #{resource_type_from} found at '#{from}'." + end + + def valid_destination? + return true if force + return true unless File.exist?(to) + + invalidate_with "A #{resource_type_to} already exists at #{to}" + end + + def invalidate_with(msg) + Jekyll.logger.warn msg + false + end + def from movement.from end diff --git a/spec/draft_spec.rb b/spec/draft_spec.rb index f5759ba..ce50a77 100644 --- a/spec/draft_spec.rb +++ b/spec/draft_spec.rb @@ -63,16 +63,16 @@ FileUtils.touch path end - it "displays a warning and abort" do + it "displays a warning and returns" do output = capture_stdout { described_class.process(args) } - expect(output).to include("A draft already exists at _drafts/an-existing-draft.md".yellow) + expect(output).to include("A draft already exists at _drafts/an-existing-draft.md") + expect(File.read(path)).to_not match("layout: post") end it "overwrites if --force is given" do - expect(lambda { - capture_stdout { described_class.process(args, "force" => true) } - }).not_to raise_error - expect(File.read(path)).to match(%r!layout: post!) + output = capture_stdout { described_class.process(args, "force" => true) } + expect(output).to_not include("A draft already exists at _drafts/an-existing-draft.md") + expect(File.read(path)).to match("layout: post") end end diff --git a/spec/page_spec.rb b/spec/page_spec.rb index a631886..1e43cb8 100644 --- a/spec/page_spec.rb +++ b/spec/page_spec.rb @@ -52,16 +52,16 @@ FileUtils.touch path end - it "displays a warning" do + it "displays a warning and returns" do output = capture_stdout { described_class.process(args) } - expect(output).to include("A page already exists at #{filename}".yellow) + expect(output).to include("A page already exists at #{filename}") + expect(File.read(path)).to_not match("layout: page") end it "overwrites if --force is given" do - expect(lambda { - capture_stdout { described_class.process(args, "force" => true) } - }).not_to raise_error - expect(File.read(path)).to match(%r!layout: page!) + output = capture_stdout { described_class.process(args, "force" => true) } + expect(output).to_not include("A page already exists at #{filename}") + expect(File.read(path)).to match("layout: page") end end diff --git a/spec/post_spec.rb b/spec/post_spec.rb index f947af0..f2652ed 100644 --- a/spec/post_spec.rb +++ b/spec/post_spec.rb @@ -74,16 +74,16 @@ FileUtils.touch path end - it "displays a warning" do + it "displays a warning and returns" do output = capture_stdout { described_class.process(args) } - expect(output).to include("A post already exists at _posts/#{filename}".yellow) + expect(output).to include("A post already exists at _posts/#{filename}") + expect(File.read(path)).to_not match("layout: post") end it "overwrites if --force is given" do - expect(lambda { - capture_stdout { described_class.process(args, "force" => true) } - }).not_to raise_error - expect(File.read(path)).to match(%r!layout: post!) + output = capture_stdout { described_class.process(args, "force" => true) } + expect(output).to_not include("A post already exists at _posts/#{filename}") + expect(File.read(path)).to match("layout: post") end end diff --git a/spec/publish_spec.rb b/spec/publish_spec.rb index 2b06695..f9ef416 100644 --- a/spec/publish_spec.rb +++ b/spec/publish_spec.rb @@ -67,11 +67,12 @@ }).to raise_error("You must specify a draft path.") end - it "errors if no file exists at given path" do + it "outputs a warning and returns if no file exists at given path" do weird_path = "_drafts/i-do-not-exist.markdown" - expect(lambda { - capture_stdout { described_class.process [weird_path] } - }).to raise_error("There was no draft found at '_drafts/i-do-not-exist.markdown'.") + output = capture_stdout { described_class.process [weird_path] } + expect(output).to include("There was no draft found at '_drafts/i-do-not-exist.markdown'.") + expect(draft_path).to exist + expect(post_path).to_not exist end context "when the post already exists" do @@ -81,18 +82,16 @@ FileUtils.touch post_path end - it "raises an error" do - expect(lambda { - capture_stdout { described_class.process(args) } - }).to raise_error("A post already exists at _posts/#{post_filename}") + it "outputs a warning and returns" do + output = capture_stdout { described_class.process(args) } + expect(output).to include("A post already exists at _posts/#{post_filename}") expect(draft_path).to exist expect(post_path).to exist end it "overwrites if --force is given" do - expect(lambda { - capture_stdout { described_class.process(args, "force" => true) } - }).not_to raise_error + output = capture_stdout { described_class.process(args, "force" => true) } + expect(output).to_not include("A post already exists at _posts/#{post_filename}") expect(draft_path).not_to exist expect(post_path).to exist end diff --git a/spec/unpublish_spec.rb b/spec/unpublish_spec.rb index 27484b4..dead7ff 100644 --- a/spec/unpublish_spec.rb +++ b/spec/unpublish_spec.rb @@ -52,11 +52,10 @@ }).to raise_error("You must specify a post path.") end - it "errors if no file exists at given path" do + it "outputs a warning and returns if no file exists at given path" do weird_path = "_posts/i-forgot-the-date.md" - expect(lambda { - capture_stdout { described_class.process [weird_path] } - }).to raise_error("There was no post found at '#{weird_path}'.") + output = capture_stdout { described_class.process [weird_path] } + expect(output).to include("There was no post found at '#{weird_path}'.") end context "when the draft already exists" do @@ -66,18 +65,16 @@ FileUtils.touch draft_path end - it "raises an error" do - expect(lambda { - capture_stdout { described_class.process(args) } - }).to raise_error("A draft already exists at _drafts/#{post_name}") + it "displays a warning and returns" do + output = capture_stdout { described_class.process(args) } + expect(output).to include("A draft already exists at _drafts/#{post_name}") expect(draft_path).to exist expect(post_path).to exist end it "overwrites if --force is given" do - expect(lambda { - capture_stdout { described_class.process(args, "force" => true) } - }).not_to raise_error + output = capture_stdout { described_class.process(args, "force" => true) } + expect(output).to_not include("A draft already exists at _drafts/#{post_name}") expect(draft_path).to exist expect(post_path).not_to exist end