Merge pull request #9 from imdrasil/fix-multiple-bugs

Add Image.build and fix a couple of bugs

README.md

@@ -10,7 +10,7 @@ Add this to your application's `shard.yml`:
 dependencies:
   crymagick:
     github: imdrasil/crymagick
-    version: 0.2.2
+    version: 0.2.3
 ```
 
 ## Requirements

shard.yml

@@ -1,10 +1,10 @@
 name: crymagick
-version: 0.2.2
+version: 0.2.3
 
 authors:
   - Roman Kalnytskyi <[email protected]>
 
-crystal: 1.0.0
+crystal: ">= 0.35.0"
 
 license: MIT
 development_dependencies:

spec/crymagick/image_spec.cr

@@ -5,6 +5,39 @@ describe CryMagick::Image do
   @subject : CryMagick::Image?
   let(:subject) { CryMagick::Image.open(image_path) }
 
+  it "create temfile" do
+    file = described_class.open("spec/fixtures/cylinder_shaded.png")
+    expect(File.exists?(file.path)).must_equal(true)
+  end
+
+  it "has attributes" do
+    expect(subject.type).must_match(/^[A-Z]+$/)
+    expect(subject.mime_type).must_match(/^image\/[a-z]+$/)
+    expect(subject.width).wont_equal(0)
+    expect(subject.height).wont_equal(0)
+    subject.dimensions
+    expect(subject.size).wont_equal(0)
+    expect(subject.human_size).wont_be_empty
+    expect_be_a(subject.colorspace, String)
+    expect_be_a(subject.resolution, Tuple(Float64, Float64))
+    expect(subject.signature).must_match(/[[:alnum:]]{64}/)
+  end
+
+  it "generates attributes of layers" do
+    expect(subject.layers[0].type).must_match(/^[A-Z]+$/)
+    expect(subject.layers[0].size > 0).must_equal true
+  end
+
+  it "changes colorspace when called with an argument" do
+    # TODO: add correct expectation
+    subject.colorspace("Gray")
+  end
+
+  it "changes size when called with an argument" do
+    # TODO: add correct expectation
+    subject.size("20x20")
+  end
+
   describe ".read" do
     it "reads image from String" do
       string = File.read(image_path)
@@ -115,6 +148,18 @@ describe CryMagick::Image do
     end
   end
 
+  describe ".build" do
+    it "chains multiple options and executes them in one command" do
+      image = described_class.build(clone_image(image_path)) do |m|
+        m.resize("20x30!")
+        m.colorspace("CMYK")
+      end
+
+      expect(image.dimensions).must_equal({20, 30})
+      expect(image.data["colorspace"].as_s).must_equal("CMYK")
+    end
+  end
+
   describe "equivalence" do
     @image : CryMagick::Image?
     @same_image : CryMagick::Image?
@@ -250,11 +295,6 @@ describe CryMagick::Image do
     end
   end
 
-  it "create temfile" do
-    file = described_class.open("spec/fixtures/cylinder_shaded.png")
-    expect(File.exists?(file.path)).must_equal(true)
-  end
-
   describe "#write" do
     it "writes the image" do
       output_path = random_path("test output")
@@ -411,34 +451,6 @@ describe CryMagick::Image do
     end
   end
 
-  it "has attributes" do
-    expect(subject.type).must_match(/^[A-Z]+$/)
-    expect(subject.mime_type).must_match(/^image\/[a-z]+$/)
-    expect(subject.width).wont_equal(0)
-    expect(subject.height).wont_equal(0)
-    subject.dimensions
-    expect(subject.size).wont_equal(0)
-    expect(subject.human_size).wont_be_empty
-    expect_be_a(subject.colorspace, String)
-    expect_be_a(subject.resolution, Tuple(Float64, Float64))
-    expect(subject.signature).must_match(/[[:alnum:]]{64}/)
-  end
-
-  it "generates attributes of layers" do
-    expect(subject.layers[0].type).must_match(/^[A-Z]+$/)
-    expect(subject.layers[0].size > 0).must_equal true
-  end
-
-  it "changes colorspace when called with an argument" do
-    # TODO: add correct expectation
-    subject.colorspace("Gray")
-  end
-
-  it "changes size when called with an argument" do
-    # TODO: add correct expectation
-    subject.size("20x20")
-  end
-
   describe "#exif" do
     let(:subject) { described_class.new(image_path(:exif)) }
 
@@ -488,8 +500,13 @@ describe CryMagick::Image do
 
   describe "#combine_options" do
     it "chains multiple options and executes them in one command" do
-      expect_to_change(->{ subject.dimensions }, to: {20, 30}) do
-        subject.combine_options(&.resize("20x30!"))
+      expect_to_change(->{ subject.data["colorspace"].as_s }, to: "Gray") do
+        expect_to_change(->{ subject.dimensions }, to: {20, 30}) do
+          subject.combine_options do |m|
+            m.resize("20x30!")
+            m.colorspace("Gray")
+          end
+        end
       end
     end
 
@@ -501,6 +518,12 @@ describe CryMagick::Image do
     it "returns self" do
       expect(subject.combine_options { }).must_equal subject
     end
+
+    it "raises an error if #format is called" do
+      assert_raises(ArgumentError) do
+        subject.combine_options(&.format("png"))
+      end
+    end
   end
 
   describe "#composite" do

spec/support/helper.cr

@@ -33,7 +33,7 @@ module Helper
       File.join("spec", "fixtures", name)
     else
       path = random_path
-      FileUtils.cp image_path, path
+      FileUtils.cp(image_path, path)
       path
     end
   end
@@ -53,4 +53,10 @@ module Helper
     array.each { |e| io.write_bytes(e) }
     io.to_s
   end
+
+  def clone_image(path)
+    new_path = random_path
+    FileUtils.cp(path, new_path)
+    new_path
+  end
 end

src/crymagick.cr

@@ -3,5 +3,5 @@ require "./crymagick/errors"
 require "./crymagick/*"
 
 module CryMagick
-  VERSION = "0.2.2"
+  VERSION = "0.2.3"
 end

src/crymagick/image.cr

@@ -22,6 +22,7 @@ module CryMagick
     # TODO: allow to pass url
     def self.open(path : String, ext : String? = nil)
       raise "File is not exists" unless File.exists?(path)
+
       ext ||= File.extname(path)
       File.open(path) { |f| read(f, ext) }
     end
@@ -54,17 +55,21 @@ module CryMagick
       target_image
     end
 
+    def self.build(path : String, tempfile = nil)
+      new(path, tempfile).combine_options { |m| yield m }
+    end
+
     getter path, tempfile : ::File?
     protected setter path
 
-    def tempfile!
-      @tempfile.not_nil!
-    end
-
     def initialize(@path : String, @tempfile = nil)
       @info = Info.new(@path)
     end
 
+    def tempfile!
+      @tempfile.not_nil!
+    end
+
     def ==(other : Image)
       signature == other.signature
     end
@@ -154,10 +159,18 @@ module CryMagick
       end
     end
 
+    # This is used to change the format of the image. That is, from "tiff to jpg" or something like that.
+    #
+    # Once you run it, the instance is pointing to a new file with a new extension!
+    #
+    # *DANGER*: This renames the file that the instance is pointing to. So, if you manually opened the file with
+    # Image.new(file_path)... Then that file is DELETED! If you used Image.open(file) then you are OK. The original
+    # file will still be there. But, any changes to it might not be...
+    #
     # page = -1 for all frames
     #
     # TODO: fix converting several frames - point current image to first one (now it points to empty img)
-    def format(_format, page : String = "0", read_options : Hash(String, String) = {} of String => String)
+    def format(_format, page : String | Int = "0", read_options : Hash(String, String) = {} of String => String)
       new_temp_file = nil
       new_path =
         if @tempfile
@@ -168,7 +181,7 @@ module CryMagick
           (parts.size == 1 ? parts[0] : parts[0...-1].join("")) + ".#{_format}"
         end
       input_path = path.clone
-      input_path += "[#{page}]" if page != "-1" && !layer?
+      input_path += "[#{page}]" if page.to_s != "-1" && !layer?
 
       Tool::Convert.build do |con|
         read_options.each do |key, value|
@@ -262,7 +275,7 @@ module CryMagick
     end
 
     def mogrify(page : Int32? = nil)
-      Tool::Mogrify.build do |builder|
+      Tool::MogrifyRestricted.build do |builder|
         yield builder
         builder << (page ? "#{path}[#{page}]" : path)
       end

src/crymagick/image/info.cr

@@ -38,6 +38,7 @@ module CryMagick
       def clear
         @info.clear
         @resolution.clear
+        @data = nil
         {% for attr in ALL_ATTRS %}
           {% if attr != "resolution" %}
             @{{attr.id}} = nil

src/crymagick/tool.cr

@@ -2,12 +2,6 @@ module CryMagick
   class Tool
     CREATION_OPERATORS = %w(xc canvas logo rose gradient radial-gradient plasma pattern label caption text pango)
 
-    def self.build(name : String) : String
-      instance = new(name)
-      yield instance
-      instance.call
-    end
-
     getter name : String, args
     @whiny : Bool
 
@@ -21,6 +15,12 @@ module CryMagick
       @whiny = options.has_key?(:whiny) ? options[:whiny] : Configuration.whiny
     end
 
+    def self.build(name : String) : String
+      instance = new(name)
+      yield instance
+      instance.call
+    end
+
     def call : String
       shell = Shell.new
       stdout = shell.run(command, {:whiny => @whiny})[0]
@@ -101,9 +101,7 @@ module CryMagick
     # Currently notification about dynamically generated methods will be printed out
     # to stdout during compilation
     macro method_missing(call)
-      {% if flag?(:crymagick_debug) %}
-        {% p "#{@type}##{call.id} is generated".id %}
-      {% end %}
+      {% flag?(:crymagick_debug) && p("#{@type}##{call.id} is generated".id) %}
       def {{call.name.id}}(*args)
         send({{call.name.tr("_", "-").id.stringify}}, *args)
       end

src/crymagick/tool/mogrify_restricted.cr

@@ -0,0 +1,9 @@
+require "./mogrify"
+
+class CryMagick::Tool
+  class MogrifyRestricted < Mogrify
+    def format(*args)
+      raise ArgumentError.new("you must call #format on a CryMagick::Image directly")
+    end
+  end
+end