-
-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for spec-2 #163
Conversation
Here are the changes I made to Cider: It adds pretty printing for |
Can you elaborate on this? Why is it hard to have it as a dep with a Lein project? |
Overall, I'm fine with the approach you've taken in this PR, but I have to admit that the current situation with the 3 versions of Spec is killing me. Let's hope at the end it will be just Spec 2 and nothing else. I find this to be extremely funny:
Naming is hard! |
It's actually super easy - one clones the repo (shallowly, of course) and adds |
src/orchard/spec.clj
Outdated
(resolve (symbol "clojure.spec.gen" ~fname)))] | ||
(f# ~@args))) | ||
(defn- spec [sym & args] | ||
(when-let [f (resolve sym)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's try require
ing before resolve
ing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using the call-when-resolved
macro we introduced for the datafy code. So now we only require/resolve once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit more verbose though :/
src/orchard/spec.clj
Outdated
(f# ~@args))) | ||
(defn- spec [sym & args] | ||
(when-let [f (resolve sym)] | ||
(try (apply f args) (catch Exception _)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This try/catch wasn't here before and doesn't seem a great idea to me...?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I did this because spec-2 is not as relaxed as the others when it comes to what types of arguments you pass to it's functions. This has been moved to the individual spec fn wrappers where I tried to mirror the behavior of the current version of spec, even though we are looking up in 3 different places.
|
||
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; | ||
;; This are all wrappers of clojure.spec.[alpha] functions. ;; | ||
;; We can't simply require the ns because it's existence depends on ;; | ||
;; clojure version ;; | ||
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; | ||
|
||
(defn get-spec [v] (spec "get-spec" v)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably the above comment should be extended to mention Spec 2 as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Here's the PR for CIDER clojure-emacs/cider#3249 |
I think this is a non-issue. :) I did what @vemv suggested. I'm checking out spec-2 via git in the Makefile and added the source dir to project.clj. I had to exclude those sources from eastwood, since it detected some lint issues in spec-2. |
Hello,
this PR adds support for spec-2 to orchard. spec-2 is supposed to be the successor of
clojure.spec.alpha
, but is still in an experimental state. The main advantage of spec-2 is that it addss/schema
ands/select
which should improve some short comings ofs/keys
from the original spec.s/schema
ands/select
are supposed to help with re-using spec declarations. You would uses/schema
to define the general shape of a data structure and then uses/select
to make concrete sub selections of specs that have been defined withs/schema
. It is not recommended to use spec-2 in production, but some people might do.When Clojure Spec was initially released it lived under the namespace
clojure.spec
. Some time after the release it was renamed toclojure.spec.alpha
. I believe because of this transition phase Orchard supports both namespaces and checks at run time which version of Clojure Spec is available and resolves specs accordingly. This PR adds a third option to the mix.The PR changes Orchard to support
spec-list
,spec-form
andspec-example
NREPL ops in the following way:For the
spec-form
andspec-example
NREPL ops the Clojure specs are resolved and generated in the following order:spec-alpha-2 (aka
clojure.alpha.spec
)spec-alpha (aka
clojure.spec.alpha
, which is shipped with Clojure these days)spec (aka
clojure.spec
, which is shipped in old Clojure versions?)There is however a chance of a conflict if a spec is declared in more than one of the registries, and multiple variants of Clojure Spec are loaded at the same time. The most likely scenario is that people who use spec-2 have loaded both spec-alpha-2 (aka
clojure.alpha.spec
) and spec-alpha (akaclojure.spec.alpha
). So spec-2 specs would shadow specs defined in a "previous" spec variant.For the
spec-list
NREPL op the available spec registries are merged with spec-2 (clojure.alpha.spec
) taking precedence over spec-alpha (akaclojure.spec.alpha
) over spec (akaclojure.spec
).This order has been chosen because I believe people using spec-2 would prefer to see those specs instead of specs form older versions, in the unlikely case of a spec defined in multiple registries.
To be honest, I'm not super confident of the "unlikely" case and what happens in the future. This was the most pragmatic approach I could think of to get spec-2 working in Cider. It seems to work in my project and it should not affect people not using spec-2.
An alternative approach could be:
We could index the results of the NREPL operations by their spec variants. So, the
spec-list
op could return:{:spec-alpha-2 ["some/spec", "another/spec"]
:spec-alpha ["some/spec"]
:spec ["some/spec"]}
And something similar for the other ops. The Cider frontend could then display all available Specs at the same time. If we want to go this direction we should probably introduces a new middleware with new ops for this, or add additional keys to the result of the existing ops. This might be the more "clean" approach, to support multiple flavors of Clojure Spec. But I'm not sure if it's worth the effort and I believe it would complicate the frontends a bit. Also I don't know how important it is to still support
clojure.spec
.Any other ideas how to best integrate this?
Another open question is:
If we want to add tests for spec-2 we somehow need to bring the dependency in. spec-2 is only available via deps.edn. We could try to add it to orchard via lein-tools-deps or use some script to fetch it and leverage Leinigen's checkout feature to somehow add it to the classpath.
WDYT?