cider-doc now renders spec using cider-browse-spec--pprint-indented

[jpmonettas]

There is also a button to jump to the browser

This needs to be merged together with! clojure-emacs/cider-nrepl#472

---

• The commits are consistent with our [contribution guidelines][1]
• 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)

[jpmonettas]

@bbatsov This is not ready to be merged yet, it works with eval-buffer but has a compilation error due to dependency cycle. I'm creating the pull request to discuss the solution.

The thing is I need to create a button in cider-doc.el that when clicked calls a procedure inside cider-browse-spec.el.

Can someone with more experience in elisp help me with this? Thanks

[xiongtx]

A screenshot would be helpful:

This presentation could be improved. See the existing:

[jpmonettas]

@xiongtx Don't know why it looks like that, mines looks different :

I prefer it to show anonymous functions as functions and not to remove the #, because they look like the spec is invalid.

Any idea how to solve that dependency cycle issue?

[xiongtx]

Hmm, I'm not sure what's causing the differences. Perhaps @bbatsov can see what results he gets.

[bbatsov]

Perhaps I'm missing something, but the first version does look a bit tidier to me. Perhaps we can make it configurable which version to display in the doc buffer? After all I guess the "Browse" button is the most important thing.

[xiongtx]

Which version looks tidier? Couldn't be the first one in my post--it doesn't even line up the args, ret, etc. We should probably disregard it--I'd guess it's b/c I didn't pull down the required version of cider-nrepl.

Anyways, I'd be fine w/ merging what @jpmonettas shows, though I don't think the s/fspec is necessary--it's just noise. That it's an fdef is obvious from the :args, :ret, etc.

[xiongtx]

Seems this can be simplified w/ cider-font-lock-as-clojure on the pprinted string.

[bbatsov]

By first, I meant the existing one. I like the nice structural distinction between args, ret and fn (and I also think we can use better labels for those, btw). My personal preference would be to show by default something easier to parse by human minds and optionally be able to show the actual spec as it exists in the code.

[jpmonettas]

@bbatsov @xiongtx Just removed the surrounding (s/fspec ...) stuff, now it looks like:

The cat looks nicer in our current version because it only has two parts, so we can show it all in one line, but as soon as you have a larger cat line I think it looks nicer to print one part in each line.

My personal preference would be to show by default something easier to parse by human minds and optionally be able to show the actual spec as it exists in the code.

I think this too, that's why I created that elisp spec formatter, so we have control and can fine tune how we print a spec instead of relaying on vanilla s/form pprinting.

Some time ago I created pretty-spec that uses fipp to pretty print specs sensitive to a width you have defined. Using something like this will require to move pprinting from elisp to cider-nrepl. (I explored those ideas some time ago when writing Inspectable

[xiongtx]

Should (require 'cider-browse-spec) at the top of the file.

[jpmonettas]

But if I do that I'm getting a dependency cycle :

In toplevel form:
cider-selector.el:35:1:Error: Recursive ‘require’ for feature ‘cider-interaction’

In toplevel form:
cider-inspector.el:32:1:Error: Recursive ‘require’ for feature ‘cider-interaction’

In toplevel form:
cider-debug.el:30:1:Error: Recursive ‘require’ for feature ‘cider-interaction’

In toplevel form:
cider-scratch.el:33:1:Error: Recursive ‘require’ for feature ‘cider-interaction’

In toplevel form:
cider-macroexpansion.el:34:1:Error: Recursive ‘require’ for feature ‘cider-interaction’

In toplevel form:
cider-interaction.el:35:1:Error: Recursive ‘require’ for feature ‘cider-repl’

In toplevel form:
cider-mode.el:35:1:Error: Recursive ‘require’ for feature ‘cider-interaction’

In toplevel form:
cider-apropos.el:28:1:Error: Recursive ‘require’ for feature ‘cider-doc’

In toplevel form:
cider-repl.el:35:1:Error: Recursive ‘require’ for feature ‘cider-doc’

In toplevel form:
cider-browse-spec.el:39:1:Error: Recursive ‘require’ for feature ‘cider-interaction’

In toplevel form:
cider-doc.el:39:1:Error: Recursive ‘require’ for feature ‘cider-browse-spec’

In toplevel form:
cider-browse-ns.el:38:1:Error: Recursive ‘require’ for feature ‘cider-interaction’

In toplevel form:
cider.el:82:1:Error: Recursive ‘require’ for feature ‘cider-repl’
Makefile:42: recipe for target 'test-bytecomp' failed

[xiongtx]

Try to figure out what's causing the dependency cycle then, and break it. We probably have some unnecessary requires in some files, or maybe some functions need to be shuffled around.

In any case, you're using cider-browse-spec--... in cider-doc.el so you should definitely require it in cider-doc.el.

[jpmonettas]

Yeah, was cider-browse-spec depending on cider-interaction, it doesn't need to, so this is fixed.

[xiongtx]

Yes, removing the s/fspec stuff cleans it up a lot. I'm good w/ how it looks now.

[jpmonettas]

Maybe this looks better? What about the naming ?

[jpmonettas]

@bbatsov @xiongtx if both agree on the latest screenshot now everything is ready to be merged together with clojure-emacs/cider-nrepl#472

• The commits are consistent with our [contribution guidelines][1]
• 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)

[bbatsov]

I'm fine with it. Just rebase on top of the current master.

[bbatsov]

You should also add some changelog entry.

[jpmonettas]

@bbatsov added an entry to the changelog and rebased on top of master.

[bbatsov]

I noticed at the build output x is unused, which I assume is a mistake. It should be just _ if it's not needed.