Alt doc for higher-order functions

[ckoparkar]

Before submitting a PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):

• The commits are consistent with our contribution guidelines
• You've added tests (if possible) to cover your change(s)
• All tests are passing (make test)
• The new code is not generating bytecode or M-x checkdoc warnings
• You've updated the changelog (if adding/changing user-visible functionality)
• You've updated the readme (if adding/changing user-visible functionality)
• You've updated the refcard (if you made changes to the commands listed there)

Thanks!

[ckoparkar]

I have checked this with these forms:

;; clojure

;; variable. should be blank
var-meta-whitelist

(defn)
(clojure.string/blank?)
(clojure.core/defn)
(str/blank?)
(cider.nrepl.middleware.ns/ns-vars-clj)
(ns/ensure-namespace)
(java/member-info (u/as-sym "java.lang.String") (u/as-sym "length"))
;; invalid. should be empty
(clojure.X/defn)

;; interop
(.length)
(java.lang.String/.length)
;; invalid. should be empty
(java.lang.X/.length)


;; combinations

(map inc [1 2 3 4 5])

(reduce + [1 2 3 4 5])

((comp inc (partial + 3)) 3)

(map :a [{:a 1 :b 2} {:a 3 :b 4}])

Please try out once :-)

[ckoparkar]

I'm not sure if this is right. But even without this var, eldoc seems to work correctly.

[ckoparkar]

I think I should remove this var. It highlights the argument, but it is subtle so I din't notice the difference. But in a sexp like (map inc ...), inc wouldn't have any arguments to highlight.
And even if the mapping function is something like + which can take 2 args, the cursor position would be such that it would never need to highlight the arguments.
Does this make sense ?

[bbatsov]

There should also be some defcustom to restore the old behaviour, as some people might prefer it.

[ckoparkar]

Ill make all the changes and update this PR.

[ckoparkar]

Done :-) All the lets still don't look pretty enough, but better than before I guess.

[Malabarba]

This comment makes it sound like nothing will be done if we're not at the beginning of sexp. Could you elaborate to make it clearer?

[bbatsov]

The highlighting of the current argument doesn't make sense for the symbol at point's docstring, but makes a lot of sense for everything else. And as far as higher order functions go I believe the idea on the ticket for them was to hardcode a list of known such functions and to have some special treatment for them. That's not something important, I'm just pointing it out.

Also now that you can get an eldoc for the variable name at point - we should font-lock the type of the eldoc and for variables we should probably show the first line of their docstring as they don't have arguments. This is how eldoc works for variables in Elisp.

[ckoparkar]

This actually doesn't get the eldoc for a variable. It only considers the (map inc ...) situation, #1638.
I'll raise another PR with those changes, for #1572.

[Malabarba]

Inline this function call so we'll don't do the request when we don't have to.

[bbatsov]

This actually doesn't get the eldoc for a variable. It only considers the (map inc ...) situation, #1638.
I'll raise another PR with those changes, for #1572.

Meaning that you'll get eldoc for the second symbol after your cursor goes pass it? Or you just get eldoc at point for whatever symbol now? I still don't quite get what exactly does the PR solve.

[ckoparkar]

@bbatsov This PR just handles what this comment describes here:

Basically we just need to make eldoc consider the symbol-at-point in addition to the first symbol of the list at point (which we currently do). So in a form like (map inc ...) if your cursor is over inc you'd see its docstring and when you move forward you'd see once again the docstring for map.

For the eldoc of any form at point, we require a small nrepl change here. I was a bit busy yesterday so I couldn't follow up on this. But I'm gonna finish both of these tickets today. I've already made the nrepl change, just have to change the elisp interface. I'll submit the nrepl PR, and the "eldoc for any form at point including variables" and all the changes @Malabarba suggested here, by tonight. :-)

[bbatsov]

OK, thanks for the update.

[ckoparkar]

I've made all the changes and tested everything. This PR now includes both, eldoc for variables and eldoc for higher order functions.

I've tried to make the functions small enough to avoid comments all together. There's probably some scope for more refactoring here, but haven't made any of those changes here.

Depends on clojure-emacs/cider-nrepl#360.

[bbatsov]

Just to clarify - the issue about alternative eldoc for higher order functions was different, so I wouldn't call this HOF-related. The idea there was in a situation like this:

(map inc [1 2 3|])

to show the eldoc for inc, not map (| denotes the cursor). This means this cannot be generic, as changing the eldoc like this doesn't make sense for every higher order function - it should be applied just for a list of know functions.

[ckoparkar]

Oh. That makes sense. The first thing to do is change the commit message probably so the issue isn't incorrectly closed. And the commit should still be merged right ?

[bbatsov]

Yep. The references you were using were the primary source of my confusion - as what was suggested in the ticket was different and I said we can simplify it if we extend my idea about variable eldoc to all symbols at point. :-)

[ckoparkar]

Yeah.. Sorry about that. Now I understand correctly 💡 . I'll change that in another PR though. This one is huge even now :-)

[ckoparkar]

I'm done with all the changes. Before uploading, wanted to change something else. Is this a little confusing ? How about we change it to var -> font-lock-variable-name-face and fn -> font-lock-function-name-face ?

I'll raise another PR for this. Would be better that way probably.

[ckoparkar]

So most of the things are handled in this PR. Found a issue with this.
The cider-eldoc-info-at-point, returns eldoc even if when the cursor is on a line which is commented out.
I want to avoid doing, go back to start of the line, check char at point and then decide. Have we implemented something similar anywhere ?

[Malabarba]

This checks if you're in a comment or a string:

(let ((s (syntax-ppss)))
  (or (nth 4 s)
      (nth 3 s)))

[bbatsov]

Just wrap the code @Malabarba shared with you in some helper function and you're good to go.

[expez]

We already have cider-in-string-p and cider-in-comment-p.

[bbatsov]

Guess we forgot about them. Good catch!

[bbatsov]

P.S. I'm a bit surprised those aren't in clojure-mode itself.

[ckoparkar]

Thanks! Ready to merge now I think.

[expez]

P.S. I'm a bit surprised those aren't in clojure-mode itself.

haha ;) I argued for that, and initially opened the PR against that repo, but you felt they belonged in CIDER.

[bbatsov]

haha ;) I argued for that, and initially opened the PR against that repo, but you felt they belonged in CIDER.

Now I remembered - my concern was that we weren't using them anywhere in clojure-mode itself.