Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn on error and exit gracefully #83

Merged
merged 1 commit into from Dec 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 8 additions & 3 deletions lib/jekyll-compose/file_creator.rb
Expand Up @@ -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
Expand All @@ -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
Expand Down
22 changes: 20 additions & 2 deletions lib/jekyll-compose/file_mover.rb
Expand Up @@ -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
Expand All @@ -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
Expand Down
12 changes: 6 additions & 6 deletions spec/draft_spec.rb
Expand Up @@ -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

Expand Down
12 changes: 6 additions & 6 deletions spec/page_spec.rb
Expand Up @@ -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

Expand Down
12 changes: 6 additions & 6 deletions spec/post_spec.rb
Expand Up @@ -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

Expand Down
21 changes: 10 additions & 11 deletions spec/publish_spec.rb
Expand Up @@ -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
Expand All @@ -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
Expand Down
19 changes: 8 additions & 11 deletions spec/unpublish_spec.rb
Expand Up @@ -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
Expand All @@ -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
Expand Down