Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also compare across forks.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also compare across forks.
base fork: neatonk/im4clj
base: master
...
head fork: bkirkbri/im4clj
compare: consolidate
Checking mergeability… Don’t worry, you can still create the pull request.
  • 2 commits
  • 5 files changed
  • 14 commit comments
  • 1 contributor
View
1  .gitignore
@@ -8,3 +8,4 @@
/.lein-deps-sum
/test/tmp
.#*
+\#*
View
15 src/im4clj/commands.clj
@@ -9,19 +9,8 @@
(ns ^{:doc "Command fn's"
:author "Kevin Neaton"}
im4clj.commands
- (:use [im4clj config run]))
-
-(defn command
- "Build a new command. Prepends \"gm\" to the command if (use-gm?) is true.
-
- Example Usage:
-
- (command :convert)
- (command 'convert)
- (command \"convert\")
- "
- [cmd]
- (if (use-gm?) (list "gm" cmd) cmd))
+ (:refer-clojure :exclude [compare import])
+ (:use [im4clj.im4java :only [run]]))
(def ^:private command-specs
{'animate
View
5 src/im4clj/core.clj
@@ -10,9 +10,8 @@
:author "Kevin Neaton"}
im4clj.core
(:use [im4clj.util ns])
- (:require [im4clj commands config options run]))
+ (:require [im4clj commands config options]))
(immigrate 'im4clj.commands
'im4clj.config
- 'im4clj.options
- 'im4clj.run)
+ 'im4clj.options)
View
87 src/im4clj/im4java.clj
@@ -9,7 +9,7 @@
(ns ^{:doc "Wrapper functions and utils for running shell commands with im4java."
:author "Kevin Neaton"}
im4clj.im4java
- (:use [im4clj config])
+ (:require [im4clj.config :as config])
(:import [org.im4java.core ImageCommand Operation]
[org.im4java.process ProcessStarter]))
@@ -24,35 +24,72 @@
(ProcessStarter/setGlobalSearchPath path)
path))
-;; core fn's
-(defn command
- "Create and return a new im4java ImageCommand from the string(s) provided."
- {:tag ImageCommand}
- [cmd & more]
- {:pre [(string? cmd) (every? string? more)]}
- (ImageCommand. (into-array String (cons cmd more))))
+;; coercian protocols
+(defprotocol ICommand
+ "An abstraction for building an ImageCommand."
+ (command [this] "Returns a function that returns an ImageCommand instance built from whatever this is."))
+
+(extend-protocol ICommand
+ String
+ (command [this] (ImageCommand. (into-array String [this])))
+ clojure.lang.Named
+ (command [this] (command (name this)))
+ ImageCommand
+ (command [this] this))
+
+(defprotocol IArgument
+ "Abstraction for building command-line arguments."
+ (stringify [this] "Returns a sequence of strings representing argument(s) this"))
+
+(extend-protocol IArgument
+ String
+ (stringify [this] (vector this))
+
+ clojure.lang.Named
+ (stringify [this] (stringify (name this)))
+ nil
+ (stringify [this] [])
+
+ clojure.lang.IPersistentCollection
+ (stringify [this] (vec (flatten (map stringify this))))
+
+ Operation
+ (stringify [this] (stringify (vec (.getCmdArgs this))))
+
+ clojure.lang.IFn
+ (stringify [this] (stringify (.invoke this)))
+
+ Object
+ (stringify [this] (stringify (str this))))
+
+;; core fn's
(defn operation
- "Create and return a new im4java Operation from the string(s) provided."
- {:tag Operation}
- [& opt-strs]
- {:pre [(every? string? opt-strs)]}
- (-> (Operation.) (.addRawArgs (into-array String opt-strs))))
-
-(defn- run*
- "Run an ImageCommand command."
- [^ImageCommand cmd ^Operation op & imgs]
- (let [imgs (->> imgs (map str) object-array)]
- (.run cmd op imgs)))
+ ""
+ [& args]
+ (-> (Operation.)
+ (.addRawArgs (into-array String (stringify args)))))
+
+(defn- adjust-for-gm
+ "Adjusts an ImageCommand to use GraphicsMagick based on the setting of use-gm?"
+ [cmd]
+ (let [cmd-strs (filter #(not (= "gm" %)) (.getCommand cmd))
+ cmd-strs (if (config/use-gm?) (cons "gm" cmd-strs) cmd-strs)]
+ ;;(doto cmd (.setCommand (into-array String cmd-strs)))
+ ;; we should be using the above, but there is a bug in im4java
+ ;; where .setCommand appends to the command instead of replacing
+ ;; it.
+ ;; For now, we create a new ImageCommand with the adjusted command
+ (ImageCommand. (into-array String cmd-strs))))
(defn run
- "Create and run an im4java ImageCommand from the string(s) provided.
+ "Create and run an im4java ImageCommand from the arg(s) provided.
Example Usage:
(run \"convert\" \"input.jpg\" \"resize\" \"100\" \"output.jpg\")"
- [cmd & op]
- {:pre [(string? cmd) (every? string? op)]}
- (let [cmd (command cmd)
- op (apply operation op)]
- (run* cmd op)))
+ [cmd & args]
+ {:pre [(satisfies? ICommand cmd)]}
+ (let [cmd (adjust-for-gm (command cmd))
+ op (operation args)]
+ (.run cmd op (into-array String []))))
View
44 src/im4clj/run.clj
@@ -1,44 +0,0 @@
-; Copyright (c) deeperbydesign, inc 2012. All rights reserved.
-; The use and distribution terms for this software are covered by the
-; Eclipse Public License 1.0 (http://opensource.org/licenses/eclipse-1.0.php)
-; which can be found in the file epl-v10.html at the root of this distribution.
-; By using this software in any fashion, you are agreeing to be bound by
-; the terms of this license.
-; You must not remove this notice, or any other, from this software.
-
-(ns ^{:doc "Stringify and run commands via im4java."
- :author "Kevin Neaton"}
- im4clj.run
- (:require [im4clj.im4java :as im4java]))
-
-(defmulti stringify-method
- "Method used by im4clj.core/stringify."
- type)
-
-(defmethod stringify-method java.lang.String [s] s)
-(defmethod stringify-method clojure.lang.Named [n] (.getName n))
-(defmethod stringify-method :default [o] (str o))
-
-(defn stringify
- "Convert args to a flat sequence of strings.
-
- TODO: define stringify method for core types and move flatten to appropriate
- methods."
- [& args]
- {:post [(coll? %)
- (every? string? %)]}
- (->> args flatten (map stringify-method)))
-
-(defn run
- "Run a command by name with the given opts. Accepts any 'stringify-able'
- type. Does not check (use-gm?).
-
- Prefer pre-defined commands e.g. im4clj.core/convert.
-
- Example Usage:
-
- (run :convert \"input.jpg\" :resize 100 \"output.jpg\")
- (run [:gm :convert] \"input.jpg\" :resize 100 \"output.jpg\")
- "
- [cmd & opts]
- (apply im4java/run (stringify cmd opts)))

Showing you all comments on commits in this comparison.

@neatonk
Owner

Great! this is what exactly i had in mind when I made setup the stringify multimethod. Thanks taking it to the protocol level and elaborating.

@neatonk
Owner

I like how using protocols simplifies this :pre condition.

@neatonk
Owner

Why not expose im4clj.run in core?

@neatonk
Owner

I like the use of stringify and protocols, and I appreciate the reduction it total lines of code. However, I wonder what is gained by combining im4clj.run into im4clj.im4java?

My goal in maintaining a strict string-only interface to im4java.run, was to allow allow the use of other commandline runners in the future... clojure.java.shell orpallet.shell. Both of these libs accept a sequence of strings arguments followed by zero or more keyword options. The code in im4clj.im4java attempts to wrap a similar interface around im4java. This interface could be extended to accept keyword options as well, as a way of configuring the ImageCommand being created. If your additions were moved to im4clj.run i would be agree with them 100%.

@neatonk
Owner

Hmm.. I think the choice between IM and GM should be made when the command string is generated. Currently this happens in im4clj.commands/command. However, if command were to return a proper Command object which implements stringify via multimethods or protocols, then the stringify method could check use-gm? at that time and act accordingly.

@bkirkbri

Definitely expose it if we keep it, but in this commit I was moving everything in im4clj.run to im4clj.im4java.

@bkirkbri

Cool. I changed it into a protocol since the multimethod was just dispatching on type. If we wanted to do something more complex, the multimethod works for me. The time spent on image processing would drown out any performance benefit gained by using protocols.

@bkirkbri

I kept going back and forth on how/when to check the use-gm? but I think you're right that extending stringify to handle ImageCommand and checking use-gm? at that time would work. But, to clarify, are you suggesting that we do NOT execute the subprocess with ImageCommand.run?

@bkirkbri

I'm game, but can you explain your reasoning for supporting multiple runners? If either of the ones you mentioned are better suited, I suppose we could move to 100% inspired by im4java and not rely on it at all. If not, then I want to be careful not to enter bike-shed territory :)

@neatonk
Owner

I'm suggesting that we could create a custom datatype called Command using defrecord. and then implement stringify for it using multimethods or protocols. This allows the use-gm logic to be command specific if necessary. This would still boil down to (.run ImageCommand ...)

(defrecord Command [cmd-seq])
(defmethod stringify-method Command
  [cmd]
  (if (use-gm?)
    (list "gm" (:cmd-seq cmd))
    (:cmd-seq cmd)))
@neatonk
Owner

Certainly, I think that im4java is the best choice right now since it supports command piping and asynchronous execution via the ProcessStarter superclass AND it's designed to be used with IM/GM. However, we haven't tested these features yet, so we don't know much about them. When we really get into it we may find that clojure.java.shell or pallet.shell are all we need OR we may decided to dig into the lower level ProcessBuilder or Runtime classes. Each of these alternatives will run a command (start a process) given a flat sequence of string arguments.

Maintaining a string-only interface while working with im4java, will give us the flexibility to change our minds and switch to a a different library later if necessary. Using strings also helps to put the rest of the code firmly in Clojure land, far away from Java and mutability.

Does this line of reasoning work for you?

@neatonk
Owner

Very funny, I've never heard that one before. You mean bike-shed as in kitchen-sink?

@bkirkbri

Nice! That's a great solution and keeps things abstracted until the moment it's run

@bkirkbri

Absolutely, I'm convinced. I think the problem for me all along was holding on to im4java as more of a base-layer than it really is. The bike-shed is an old programmer joke that I might have fallen into already :)

Something went wrong with that request. Please try again.