Skip to content
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

dynamic cljs completions with suitable #633

Merged
merged 1 commit into from
Aug 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* [#605](https://github.com/clojure-emacs/cider-nrepl/pull/605): Added a option for filtering vars to the ns-vars middleware.
* Added `xref` middleware providing `fn-deps` and `fn-refs` ops.
* [#628](https://github.com/clojure-emacs/cider-nrepl/pull/628): Added `clojuredocs` middleware providing `clojuredocs-lookup` and `clojuredocs-refresh-cache` ops.
* [#633](https://github.com/clojure-emacs/cider-nrepl/pull/633) Added runtime code completion for ClojureScript via [suitable](https://github.com/rksm/clj-suitable).

### Bugs fixed

Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ Let's also acknowledge some of the projects leveraged by cider-nrepl:
* [orchard][] - extracted from `cider-nrepl`, so that non-nREPL clients can leverage the generic tooling functionality (like `inspect`, `apropos`, `var-info`, etc
* [compliment][] - for Clojure code completion
* [cljs-tooling][] - for ClojureScript code completion
* [suitable][] - for ClojureScript code completion using runtime inspection
* [tools.trace][] - for tracing
* [tools.namespace][] - for namespace reloading
* [cljfmt][] - for code formatting
Expand All @@ -402,6 +403,7 @@ Distributed under the Eclipse Public License, the same as Clojure.
[orchard]: https://github.com/clojure-emacs/orchard
[compliment]: https://github.com/alexander-yakushev/compliment
[cljs-tooling]: https://github.com/clojure-emacs/cljs-tooling
[suitable]: https://github.com/rksm/clj-suitable
[tools.trace]: https://github.com/clojure/tools.trace
[tools.namespace]: https://github.com/clojure/tools.namespace
[cljfmt]: https://github.com/weavejester/cljfmt
Expand Down
1 change: 1 addition & 0 deletions project.clj
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
^:inline-dep [fipp "0.6.18"] ; can be removed in unresolved-tree mode
^:inline-dep [compliment "0.3.9"]
^:inline-dep [cljs-tooling "0.3.1"]
^:inline-dep [org.rksm/suitable "0.2.1" :exclusions [org.clojure/clojurescript]]
^:inline-dep [cljfmt "0.6.4" :exclusions [org.clojure/clojurescript]]
^:inline-dep [org.clojure/tools.namespace "0.3.0"]
^:inline-dep [org.clojure/tools.trace "0.7.10"]
Expand Down
12 changes: 9 additions & 3 deletions src/cider/nrepl/middleware/complete.clj
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,22 @@
[cljs-tooling.complete :as cljs-complete]
[compliment.core :as jvm-complete]
[compliment.utils :as jvm-complete-utils]
[orchard.misc :as u]))
[orchard.misc :as u]
[suitable.complete-for-nrepl :as suitable]))

(defn- cljs-complete
[msg cljs-env ns prefix extra-metadata]
(concat (cljs-complete/completions cljs-env prefix {:context-ns ns
:extra-metadata extra-metadata})
(suitable/complete-for-nrepl msg)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does suitable need the entire msg? I'm asking mostly because it seems to me that all the other params should be enough to get candidates.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It needs access to the cljs options + repl env + compiler env. We can pull those out and just pass them along but it seemed simpler to hand in the msg.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. As this is something minor I guess there's no need to fret over it too much.


(defn complete
[{:keys [ns symbol context extra-metadata] :as msg}]
(let [ns (u/as-sym ns)
prefix (str symbol)
extra-metadata (set (map keyword extra-metadata))]
(if-let [cljs-env (cljs/grab-cljs-env msg)]
(cljs-complete/completions cljs-env prefix {:context-ns ns
:extra-metadata extra-metadata})
(cljs-complete msg cljs-env ns prefix extra-metadata)
(jvm-complete/completions prefix {:ns ns
:context context
:extra-metadata extra-metadata}))))
Expand Down
20 changes: 20 additions & 0 deletions test/cljs/cider/nrepl/middleware/cljs_complete_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,26 @@
(is (= '("[psym & doc+methods]") (:arglists candidate)))
(is (string? (:doc candidate))))))

(deftest cljs-complete-with-suitable-test
(testing "js global completion"
(let [response (session/message {:op "complete"
:ns "cljs.user"
:symbol "js/Ob"})
candidates (:completions response)]
(is (= [{:candidate "js/Object", :ns "js", :type "function"}] candidates))))

(testing "manages context state"
(session/message {:op "complete"
:ns "cljs.user"
:symbol ".xxxx"
:context "(__prefix__ js/Object)"})
(let [response (session/message {:op "complete"
:ns "cljs.user"
:symbol ".key"
:context ":same"})
candidates (:completions response)]
(is (= [{:ns "js/Object", :candidate ".keys" :type "function"}] candidates)))))

(deftest cljs-complete-doc-test
(let [response (session/message {:op "complete-doc" :symbol "tru"})]
(is (= (:status response) #{"done"}))
Expand Down