-
-
Notifications
You must be signed in to change notification settings - Fork 646
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
Spec2 #3249
Spec2 #3249
Conversation
c5dd026
to
291d1c6
Compare
You might consider also extending the docs here a little bit https://docs.cider.mx/cider/usage/misc_features.html#browsing-the-clojure-spec-registry and mention that Spec 2 is now also supported, plus some of the pitfalls of mixing spec versions. Potentially we can move the Spec stuff to a dedicated page if we end up having more spec-relation functionality. |
(string-join | ||
(thread-last | ||
(seq-sort-by #'car #'string< name-spec-pairs) | ||
(mapcar (lambda (s) (concat (cl-first s) " " (cider-browse-spec--pprint (cl-second s)))))) |
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.
cl-first
is basically car
. :-)
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 leave all those cl-
vs nth
for now? I think it is more consistent with the rest of the code in this namespace, since cl-first
to cl-third
are actually used a LOT in this namespace. ;) WDYT?
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.
Fair enough, I guess I didn't pay enough attention to this bit of code when it was submitted initially.
|
||
(defun cider-browse-spec--render-schema (spec-form) | ||
"Render the s/schema SPEC-FORM." | ||
(let ((schema-args (cl-second spec-form))) |
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 prefer the use of nth
over forms like cl-second
.
(defun cider-browse-spec--render-select (spec-form) | ||
"Render the s/select SPEC-FORM." | ||
(let ((keyset (cl-second spec-form)) | ||
(selection (cl-third spec-form))) |
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.
Ditto.
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 don't really mind, but one advantage I see in using the cl-
functions is, your mind does not need to care about the additional index parameter at all. And maybe you save a cons cell :)
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 just don't like mixing APIs. For me cl-lib
is mostly a remnant of the past and its rarely needed these days, given many advancements that happened in the core Elisp APIs. But this is definitely not a big deal and I didn't spend any time on a focused effort to expunge cl-lib
from any project.
@bbatsov I added a section about Clojure Spec versions to the docs. Regarding the use of the |
Yeah, that's fine. |
@bbatsov This one would be ready from my side. Would you like to take another look please? |
I think the code is in a good shape, but we need to cut new orchard and cider-nrepl releases before I can merge this PR. Now that we've decided how to proceed we'll |
This PR add support for rendering the following Clojure Spec 2 forms:
Support for Clojure Spec 2 has been added to the Orchard here:
clojure-emacs/orchard#163
Before submitting the PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):
eldev test
)eldev lint
) which is based onelisp-lint
and includescheckdoc
, check-declare, packaging metadata, indentation, and trailing whitespace checks.