Removing restarts from v; dynamic variables and binding already do th…

…e job.

DESIGN.md

@@ -15,7 +15,7 @@ Vars:
 
 Skipping helped distinguish the approaches; I was able to add it with keywords while preserving (most of) the API. I have an idea on [how to do it with vars](https://github.com/hanjos/conditio-clj/commit/aa5ccff07ea56b0cf00015463701efd74df981fd), but it relies on Java reflection to access Clojure's innards, and I didn't see any other way. That's a complex and _fragile_ solution.
 
-**Decision**: I'll go with keywords, and remove the vars "fork".
+**Decision:** I'll go with keywords, and remove the vars "fork".
 
 ## 0.3.0
 
@@ -25,7 +25,10 @@ I realized my previous attempt was complicating things more than needed, and som
 
 In the first attempt, `defcondition`/`defrestart` acted more like default handlers, set up to be "rebound" by `binding`s. Now, they are more like specialized `signal`lers, in that they signal themselves when called. So one doesn't rebind them with `binding`; instead, one uses special constructs (`handle` and `with`) to do the job.
 
-Now that I have something (apparently!) working, there's some stuff to mull over:
+Some stuff to mull over:
 * Should I go with keywords or vars? I kinda like vars a little more; the need to `def` the conditions and restarts helps documentation, and folks tend to skimp on that...
-* There's very little restart code that doesn't basically reimplement dynamic variable machinery; `restart-not-found` checks, and a convenient map of available restarts in `*restarts*`. If I find a way around those, or decide they don't carry their weight, then I could remove pretty much all restart machinery (`with`, `restart`...). Hum...
+* Turns out, there's very little restart code that doesn't basically reimplement dynamic variable machinery; `restart-not-found` checks, and a convenient map of available restarts in `*restarts*`. If I find a way around those, or decide they don't carry their weight, then I could remove pretty much all restart machinery (`with`, `restart`...). Hum...
+
+**Decision:** I removed all restart machinery: dynamic variables and `binding` already do the job. That being said, `v/bind-fn` usage should be common enough to be helpful.
+
 

src/org/sbrubbles/conditio/vars.clj

@@ -1,15 +1,16 @@
 (ns org.sbrubbles.conditio.vars
   "A variation on `org.sbrubbles.conditio`, which uses vars instead of keywords.
+  There is no machinery for restarts; dynamic variables cover all intended uses.
 
   Example usage:
   ```clojure
   (require '[org.sbrubbles.conditio.vars :as v])
 
   (v/defcondition condition)
-  (v/defrestart restart)
+  (def ^:dynamic *restart* (partial v/abort \"No restart defined!\"))
 
-  (v/handle [condition #(restart %)]
-    (v/with [restart inc]
+  (v/handle [condition #(*restart* %)]
+    (binding [*restart* inc]
       (assert (= (condition 1)
                  2))))
   ```")
@@ -31,7 +32,6 @@
   SKIP)
 
 (declare handler-not-found)
-(declare restart-not-found)
 
 (def ^:dynamic *handlers*
   "A map with the available handlers, stored as handler chains. Use `handle`
@@ -40,16 +40,7 @@
   A handler is a function which takes any number of arguments and returns the
   end result. A handler chain is a list of handlers, to be run one at a time
   until the first non-`(skip)` value is returned."
-  {#'handler-not-found (list (partial abort #'handler-not-found))
-   #'restart-not-found (list (partial abort #'restart-not-found))})
-
-(def ^:dynamic *restarts*
-  "A map with the available restarts. Use `with` to install new restarts, and
-  `restart` to run them.
-
-  A restart is a function which recovers from conditions, expected to be called
-  from a handler."
-  {})
+  {#'handler-not-found (list (partial abort #'handler-not-found))})
 
 (defn- run-handler
   "Runs handler chains, returning the first non-(skip) value."
@@ -89,8 +80,6 @@
 
 (defcondition handler-not-found
   "A condition which signals when a handler couldn't be found.")
-(defcondition restart-not-found
-  "A condition which signals when a restart couldn't be found.")
 
 (defn- var-ize
   "For binding-style macros. Converts pairs into a map."
@@ -131,38 +120,8 @@
     {#'*handlers* (merge-bindings *handlers* binding-map)}
     (fn [] (bound-fn* f))))
 
-(defn restart
-  "Searches for a restart mapped to `v`, and then runs it with `args`.
-
-  Signals `restart-not-found` if no restart could be found."
-  [v & args]
-  (if-let [r (get *restarts* v)]
-    (apply r args)
-    (restart-not-found {:condition v :args args})))
-
-(defmacro defrestart
-  "Creates a restart: basically, a function which `restart`s itself when
-  called. `metadata` is expected to be either a doc-string or a metadata map."
-  ([v] `(defrestart ~v {}))
-  ([v metadata]
-   `(def ~(with-meta v (merge (->metadata metadata)
-                              {:arglists `'([& ~'args])}))
-      (partial restart (var ~v)))))
-
-(defmacro with
-  "Installs the given bindings in `*restarts*`, executes `body`, and returns
-  its result."
-  [bindings & body]
-  `(binding [*restarts* (merge *restarts*
-                               (hash-map ~@(var-ize bindings)))]
-     ~@body))
-
-(defn with-fn
-  "Returns a function, which will install the given restarts and then run `f`.
-
-  This may be used to define a helper function which runs on a different
-  thread, but needs the given restarts in place."
+(defn bind-fn
+  "Returns a function, which will install the bindings in `binding-map`
+   and then call `f` with the given arguments."
   [binding-map f]
-  (with-bindings*
-    {#'*restarts* (merge *restarts* binding-map)}
-    (fn [] (bound-fn* f))))
+  (with-bindings* binding-map (fn [] (bound-fn* f))))

test/org/sbrubbles/conditio/examples/vars_parse_log_entry_test.clj

@@ -6,21 +6,21 @@
     (clojure.lang ExceptionInfo)))
 
 (v/defcondition malformed-log-entry)
-(v/defrestart retry-with)
-(v/defrestart skip-entry)
+(def ^:dynamic *retry-with*)
+(def ^:dynamic *skip-entry*)
 
-(def ^:dynamic *selected-handler* skip-entry)
+(def ^:dynamic *selected-handler* *skip-entry*)
 
 ;; functions
 
 (defn parse-log-entry [line]
   (if (not (= line :fail))
     (str ">>> " line)
-    (v/with [retry-with parse-log-entry]
+    (binding [*retry-with* parse-log-entry]
       (malformed-log-entry line))))
 
 (defn parse-log-file []
-  (comp (map (v/with-fn {#'skip-entry (fn [] ::skip-entry)}
+  (comp (map (v/bind-fn {#'*skip-entry* (fn [] ::skip-entry)}
                         parse-log-entry))
         (filter #(not (= % ::skip-entry)))))
 
@@ -39,7 +39,7 @@
 (deftest analyze-logs-test
   (are [f input expected] (= (apply f input) expected)
        ; everything except :fail is parsed
-       (select-handler analyze-logs (fn [_] (skip-entry)))
+       (select-handler analyze-logs (fn [_] (*skip-entry*)))
        [["a" "b"] ["c" :fail :fail] [:fail "d" :fail "e"]]
        [">>> a" ">>> b" ">>> c" ">>> d" ">>> e"]
 
@@ -49,7 +49,7 @@
        [">>> a" ">>> b" ">>> c" "X" "X" "X" ">>> d" "X" ">>> e"]
 
        ; :fail's are reparsed with "X" as input instead
-       (select-handler analyze-logs (fn [_] (retry-with "X")))
+       (select-handler analyze-logs (fn [_] (*retry-with* "X")))
        [["a" "b"] ["c" :fail :fail] [:fail "d" :fail "e"]]
        [">>> a" ">>> b" ">>> c" ">>> X" ">>> X" ">>> X" ">>> d" ">>> X" ">>> e"]))
 

test/org/sbrubbles/conditio/vars_test.clj

@@ -23,11 +23,8 @@
          (v/skip 1 2 3)
          (v/skip))))
 
-(deftest handlers-and-restarts
-  (is (contains? v/*handlers* #'v/handler-not-found))
-  (is (contains? v/*handlers* #'v/restart-not-found))
-
-  (is (empty? v/*restarts*)))
+(deftest initial-handlers
+  (is (contains? v/*handlers* #'v/handler-not-found)))
 
 (v/defcondition c)
 (v/defcondition c-with-doc "doc")
@@ -52,28 +49,6 @@
        #'c-with-obj :sbrubbles nil
        #'c-with-obj :metadata 42))
 
-(v/defrestart r)
-(v/defrestart r-with-doc "doc")
-(v/defrestart r-with-meta {:sbrubbles 1 :doc "sbrubbles"})
-(v/defrestart r-with-obj 42)
-
-(deftest defrestart-metadata
-  (are [v field val] (= (field (meta v)) val)
-       #'r :doc nil
-       #'r :sbrubbles nil
-       #'r :metadata nil
-
-       #'r-with-doc :doc "doc"
-       #'r-with-doc :sbrubbles nil
-       #'r-with-doc :metadata nil
-
-       #'r-with-meta :doc "sbrubbles"
-       #'r-with-meta :sbrubbles 1
-       #'r-with-meta :metadata nil
-
-       #'r-with-obj :doc nil
-       #'r-with-obj :sbrubbles nil
-       #'r-with-obj :metadata 42))
 
 (deftest handling
   (is (thrown? ExceptionInfo (c 1 2 3)))
@@ -103,41 +78,15 @@
 
   (is (thrown? ExceptionInfo (c 1 2 3))))
 
-(deftest restarting
-  (is (thrown? ExceptionInfo (r 1 2 3)))
-
-  (testing "calling r is the same as calling v/restart"
-    (v/with [r list]
-      (is (= (v/*restarts* #'r)
-             list))
-
-      (is (= (v/restart #'r 1 2 3)
-             (r 1 2 3)
-             (list 1 2 3)))))
-
-  (testing "v/with-fn and v/with"
-    (let [r-prime (v/with-fn {#'r list}
-                             (fn [& args] (apply r args)))]
-      (is (= (r-prime 1 2 3)
-             (list 1 2 3)))
-
-      (is (thrown? ExceptionInfo (r 1 2 3))))
-
-    (v/with [r v/abort]
-      (let [r-prime (v/with-fn {#'r list}
-                               (fn [& args] (apply r args)))]
-        (is (= (r-prime 1 2 3)
-               (list 1 2 3))))))
-
-  (is (thrown? ExceptionInfo (r 1 2 3))))
+(def ^:dynamic *r* (partial v/abort "No restart defined!"))
 
-(deftest handle-and-with
-  (v/handle [c #(r %)]
-    (v/with [r inc]
-      (is (= (c 1) 2))))
+(deftest handle-with-restarts
+  (v/handle [c #(*r* %)]
+            (binding [*r* inc]
+              (is (= (c 1) 2))))
 
-  (v/with [r inc]
-    (v/handle [c #(r %)]
+  (binding [*r* inc]
+    (v/handle [c #(*r* %)]
       (is (= (c 1) 2)))))
 
 (deftest skipping-handlers