Allow predicates in plugin and middleware lists #2238
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -157,8 +157,10 @@ version from the CIDER package or library.") | |
| :package-version '(cider . "0.17.0")) | ||
|
|
||
| (defcustom cider-clojure-cli-parameters | ||
| "-e '(require (quote cider-nrepl.main)) (cider-nrepl.main/init %s)'" | ||
| "Params passed to clojure to start an nREPL server via `cider-jack-in'. | ||
| This is evaluated using `format', with the first argument being the Clojure | ||
| vector of middleware variables as a string." | ||
| :type 'string | ||
| :group 'cider | ||
| :safe #'stringp | ||
|
|
@@ -346,7 +348,14 @@ Throws an error if PROJECT-TYPE is unknown. Known types are | |
| (pcase project-type | ||
| ("lein" cider-lein-parameters) | ||
| ("boot" cider-boot-parameters) | ||
| ("clojure-cli" (format cider-clojure-cli-parameters | ||
| (concat | ||
| "[" | ||
| (mapconcat | ||
| (apply-partially #'format "\"%s\"") | ||
| (cider-jack-in-normalized-nrepl-middlewares) | ||
| ", ") | ||
| "]"))) | ||
| ("shadow-cljs" cider-shadow-cljs-parameters) | ||
| ("gradle" cider-gradle-parameters) | ||
| (_ (user-error "Unsupported project type `%s'" project-type)))) | ||
|
|
@@ -385,17 +394,60 @@ specifying the artifact ID, and the second element the version number." | |
| (string :tag "Version")))) | ||
|
|
||
| (defvar cider-jack-in-lein-plugins nil | ||
| "List of Leiningen plugins to be injected at jack-in. | ||
| Each element is a list of artifact name and version, followed optionally by | ||
| keyword arguments. The only keyword argument currently accepted is | ||
| `:predicate', which should be given a function that takes the list (name, | ||
| version, and keyword arguments) and returns non-nil to indicate that the | ||
| plugin should actually be injected. (This is useful primarily for packages | ||
| that extend CIDER, not for users. For example, a refactoring package might | ||
| want to inject some middleware only when within a project context.)") | ||
| (put 'cider-jack-in-lein-plugins 'risky-local-variable t) | ||
| (cider-add-to-alist 'cider-jack-in-lein-plugins | ||
| "cider/cider-nrepl" (upcase cider-version)) | ||
|
|
||
| (defun cider-jack-in-normalized-lein-plugins () | ||
| "Return a normalized list of Leiningen plugins to be injected. | ||
| See `cider-jack-in-lein-plugins' for the format, except that the list | ||
| returned by this function does not include keyword arguments." | ||
| (thread-last cider-jack-in-lein-plugins | ||
| (seq-filter | ||
| (lambda (spec) | ||
| (if-let* ((pred (plist-get (seq-drop spec 2) :predicate))) | ||
| (funcall pred spec) | ||
| t))) | ||
| (mapcar | ||
| (lambda (spec) | ||
| (seq-take spec 2))))) | ||
|
|
||
| (defvar cider-jack-in-nrepl-middlewares nil | ||
| "List of Clojure variable names. | ||
| Each of these Clojure variables should hold a vector of nREPL middlewares. | ||
| Instead of a string, an element can be a list containing a string followed | ||
| by optional keyword arguments. The only keyword argument currently | ||
| accepted is `:predicate', which should be given a function that takes the | ||
| list (string and keyword arguments) and returns non-nil to indicate that | ||
| the middlewares should actually be injected.") | ||
| (put 'cider-jack-in-nrepl-middlewares 'risky-local-variable t) | ||
| (add-to-list 'cider-jack-in-nrepl-middlewares "cider.nrepl/cider-middleware") | ||
|
|
||
| (defun cider-jack-in-normalized-nrepl-middlewares () | ||
| "Return a normalized list of middleware variable names. | ||
| See `cider-jack-in-nrepl-middlewares' for the format, except that the list | ||
| returned by this function only contains strings." | ||
| (thread-last cider-jack-in-nrepl-middlewares | ||
| (seq-filter | ||
| (lambda (spec) | ||
| (or (not (listp spec)) | ||
| (if-let* ((pred (plist-get (cdr spec) :predicate))) | ||
| (funcall pred spec) | ||
| t)))) | ||
| (mapcar | ||
| (lambda (spec) | ||
| (if (listp spec) | ||
| (car spec) | ||
| spec))))) | ||
|
|
||
| (defun cider--list-as-boot-artifact (list) | ||
| "Return a boot artifact string described by the elements of LIST. | ||
| LIST should have the form (ARTIFACT-NAME ARTIFACT-VERSION). The returned | ||
|
|
@@ -523,14 +575,14 @@ dependencies." | |
| (cider-add-clojure-dependencies-maybe | ||
| cider-jack-in-dependencies) | ||
| cider-jack-in-dependencies-exclusions | ||
| (cider-jack-in-normalized-lein-plugins))) | ||
| ("boot" (cider-boot-jack-in-dependencies | ||
| global-opts | ||
| params | ||
| (cider-add-clojure-dependencies-maybe | ||
| cider-jack-in-dependencies) | ||
| (cider-jack-in-normalized-lein-plugins) | ||
|
There was a problem hiding this comment. I guess this is some odd legacy, but why are we passing lein plugins to boot? //cc @benedekfazekas There was a problem hiding this comment. because they are implemented as such (both cider-nrepl and refactor-nrepl). boot either has machinery to handle them or just handles them as ordinary deps. There was a problem hiding this comment. Yeah, I know why it was done. :-) I'm just saying it looks really strange to be passing something named lein-plugins to boot. For something like tools.deps and shadow-cljs the middleware packages would be regular deps. There was a problem hiding this comment. right. unfortunately lein needs them separately. this is unfortunately the smallest denominator in terms of interface. have not checked the details of this PR yet but I am sure this can be improved. |
||
| (cider-jack-in-normalized-nrepl-middlewares))) | ||
| ("clojure-cli" (cider-clojure-cli-jack-in-dependencies | ||
| global-opts | ||
| params | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 reasonably certain without some examples most people would be really confused what are they supposed to put here.
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.
@bbatsov Do you mean in terms of the format of the keyword arguments, or why you would want to use a predicate?
If the latter, I'm not sure that this is really meant to be an end-user feature. The only current application is in
clj-refactor.el, after all. But I would not at all be opposed to adding examples.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 see. That makes sense, but it's exactly the kind of info that has to be documented. First I thought this might be something users would want to use in general or something.
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 guess the right solution would be just add some logic that removes those plugins from the deps if they are present there and add
cider-nreplto the list of default deps where it belong. Then we stop using this variable for non lein projects.