Refactor doseq to look like clojure's #503
Conversation
There was a problem hiding this comment.
Thanks for the PR! Apologies for the delay. I have some questions and requests, but I appreciate the help. 🙂
| ([ref timeout-ms timeout-val])) | ||
| ;(if (instance? clojure.lang.IBlockingDeref ref) | ||
| ; (.deref ^clojure.lang.IBlockingDeref ref timeout-ms timeout-val) | ||
| ; (deref-future ref timeout-ms timeout-val)) | ||
| )) | ||
|
|
There was a problem hiding this comment.
Looks like the entire file has been formatted. Please only change the lines related to doseq.
| (defn ^:private assert-arguments | ||
| [& pairs] | ||
| (let [pairs (partition 2 pairs)] | ||
| (loop [ps pairs] | ||
| (when (seq ps) | ||
| (let [[pred msg] (first ps)] | ||
| (when (not pred) | ||
| (throw (str msg)))) | ||
| (recur (next ps)))))) |
There was a problem hiding this comment.
I believe this is a duplicate of assert-macro-args, already in this file.
| bindings and filtering as provided by \"for\". Does not retain | ||
| the head of the sequence. Returns nil." | ||
| [seq-exprs & body] | ||
| (assert-arguments | ||
| (vector? seq-exprs) "doseq requires a vector for its binding" | ||
| (even? (count seq-exprs)) "doseq requires an even number of forms in binding vector") | ||
| (let [[_ form] (emit-doseq-step nil (seq seq-exprs) body)] | ||
| form)) |
There was a problem hiding this comment.
You mentioned this had to be broken up into separate functions. Why is that? If there's an issue preventing us from using the code that Clojure has, we should identify it so it can be fixed.
There was a problem hiding this comment.
@jeaye It was broken for clarity, the huge macro was difficult to follow. I don't remember the issue thought
There was a problem hiding this comment.
Ok, cool. Let's keep it as close to Clojure as we possibly can. This will make porting over new changes in Clojure much easier, since they'll be 1:1.
There was a problem hiding this comment.
@jeaye doseq updated from clojure core, only tiny adjasments made
There was a problem hiding this comment.
Thanks! The PR is showing a huge diff right now, as well as conflicts. It might we easier to spin up a new one or rebase from main, so we can actually just see the doseq changes.
This implements doseq
Had to brake it done to smaller functions. But is pretty close to the original.
Semantics are followed