Make cider-doc use cider-browse-spec functionality to print the spec part of the doc buffer

[jpmonettas]

Use the new functionality in cider-browse-spec to pretty print the specs in the doc buffer so we use the same printing rules as in the spec browser.

The function to use is cider-browse-spec--pprint https://github.com/clojure-emacs/cider/blob/master/cider-browse-spec.el#L132

Related issues: clojure-emacs/cider-nrepl#425

[jpmonettas]

I already added functionality for this, and works fine but when figuring out why we aren't showing specs for macros in cider-doc I found a couple of issues so I'll better ask first.

First issue is with info-clj calling m/resolve-var

(or
    ;; it's a special (special-symbol? or :special-form)
    (m/special-sym-meta sym)
    ;; it's a var
    (m/var-meta (m/resolve-var ns sym))
    ;; sym is an alias for another ns
    .....)

(defn var-meta
  ...
   (let [meta-map (-> (or (special-sym-meta v) ;; NEVER GOING TO BE CALLED
                                      (meta v))
                                 maybe-protocol
                                 (select-keys (or whitelist var-meta-whitelist))
                      map-seq maybe-add-file)]
     (maybe-add-spec v meta-map))))

I think this code is supposed to treat macros also, but calling (info-clj 'clojure.core 'let) wiill take the special-sym-meta path which will not add specs.

Changing the order of those lines in info-clj so we first check if its a var found an issue with meta/resolve-var

(m/resolve-var 'clojure.core 'Enum) => java.lang.Enum
(var? (m/resolve-var 'clojure.core 'Enum)) => false

Can we implement m/resolve-var like

(defn resolve-var
  [ns sym]
  (find-var (symbol (name ns) (name sym))))

Please let me know your thoughts

[jpmonettas]

@bbatsov @expez @arichiardi any thoughts on this?

[arichiardi]

I am not familiar with the internals, but doc should already do this. If we don't call doc directly then yes that would be awesome! -- Sent from my Android device with K-9 Mail. Please excuse my brevity.

[bbatsov]

I've been crazy busy lately. Hopefully I'll find some time for this tomorrow.

[bbatsov]

@jpmonettas I'm fine with the changes you've suggested. @xiongtx also brought up recently the point that something can be both a special form and a macro, so it such situation we should favour the macro execution code path I guess.

[bbatsov]

@jpmonettas Ping :-)

[bbatsov]

@jpmonettas Ping 2 :-)

[bbatsov]

@xiongtx Seems @jpmonettas won't be finishing this. Would you be interested in taking over the task, given the fact you're familiar with this part of the codebase? (I'd to clear the backlog a bit and I certainly need help for this to happen)

[xiongtx]

I'm traveling today but I'll take a look this week.

[bbatsov]

@xiongtx Thanks!

[jpmonettas]

@bbatsov @xiongtx hey guys sorry, I have been pretty busy with other stuff. I'm traveling but will be at home tomorrow and can take a look. Happy New year!

[xiongtx]

It's not clear to me that the cider-browse-spec--pprint form is always better. E.g.

(defn ranged-rand
  "Returns random int in range start <= rand <= end"
  [start end]
  (+ start (long (rand (- end start)))))

(s/fdef ranged-rand
        :args (s/and (s/cat :start int? :end int?)
                     #(< (:start %) (:end %)))
        :ret int?
        :fn (s/and #(>= (:ret %) (-> % :args :start))
                   #(< (:ret %) (-> % :args :end))))

cider-var-info gives:

while cider-browse-spec gives:

I'd say the former is superior.

IIUC the specs we expect to find via cider-doc are usually s/fdefs, since cider-doc doesn't work on keywords. While it is possible to s/def symbols instead of keywords, it's unconventional.

[jpmonettas]

@xiongtx that's because there's no rule for fn* to display it pretty in cider-browse-spec--pprint, should be easy to add.
The difference is that the last is browsable while the current is just a string.

[xiongtx]

I'm don't think making the spec portion of cider-doc link to the spec browser would make for good UX. For one thing, the help xrefs are not able to jump back to the cider-doc buffer.

I'll let you take a crack at this since you say you've already added functionality for it.

[jpmonettas]

@bbatsov @xiongtx I think two issues got mixed here.

One is fixing info-clj (in cider-nrepl) so it correctly returns doc info for core macros for which I created (clojure-emacs/cider-nrepl#468)

The other is (if we want) to use cider-browse-spec--pprint to print the spec part of the doc buffer so it has the format and links of the spec browser and the ability to navigate deeper into the specs using the spec browser. For me this is practical but agree it's maybe not the best UX design.

[bbatsov]

One is fixing info-clj (in cider-nrepl) so it correctly returns doc info for core macros for which I created (clojure-emacs/cider-nrepl#468)

Yeah, this should definitely be fixed.

The other is (if we want) to use cider-browse-spec--pprint to print the spec part of the doc buffer so it has the format and links of the spec browser and the ability to navigate deeper into the specs using the spec browser. For me this is practical but agree it's maybe not the best UX design.

That can always be configurable (the true spirit of Emacs), or even better - it can just be an extra button in the doc buffer "Browser Spec" (or something like this), which just switches to the spec browser.

@xiongtx that's because there's no rule for fn* to display it pretty in cider-browse-spec--pprint, should be easy to add.

Guess this should be done regardless of the decision on the previous point. @xiongtx is right that the spec browser version looks a bit convoluted.

IIUC the specs we expect to find via cider-doc are usually s/fdefs, since cider-doc doesn't work on keywords. While it is possible to s/def symbols instead of keywords, it's unconventional.

@xiongtx Good point.

[jpmonettas]

For fixing #2029 (comment) just created clojure-emacs/cider-nrepl#470

[arichiardi]

Hello folks, what is the status of this one? I am asking because I have a problem - in ClojureScript I cannot see the spec part displayed in cider-doc. Is it showing in Clojure-land?

[arichiardi]

I can confirm my problem is in ClojureScript land. In Clojure I see it. Well done BTW!

[jpmonettas]

@arichiardi it doesn't seem implemented on nrepl side for clojurescript yet (https://github.com/clojure-emacs/cljs-tooling/blob/master/src/cljs_tooling/info.cljc)

It is implemented for clojure in (https://github.com/clojure-emacs/orchard/blob/master/src/orchard/meta.clj#L40-L46)

[arichiardi]

Thanks for digging @jpmonettas and yeah ... I saw that too and I am wondering if it makes sense to merge all the things into orchard at this point

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

[stale]

This issues been automatically closed due to lack of activity. Feel free to re-open it if you ever come back to it.