Warn on error and exit gracefully (#83)

Merge pull request 83

Author: ashmaroli

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

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

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
 

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
 

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
 

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

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