Implement pretty-printing support in the nREPL middleware

[bbatsov]

Piggieback 0.5 added an implementation of nREPL's result printing functionality. It'd be nice of shadow supported this as well. Here's the relevant Piggieback PR nrepl/piggieback#108

I got reminded about this, as a user today asked over Slack why isn't pretty-printing working with shadow.

[thheller]

Can you save me some time and tell me what is expected at the nrepl message protocol level?

Something like "if the nrepl eval message contains KV (eg. :pprint true) the eval result message should contains extra keys X with Y being the pprinted value string"?

That would be great to have for every message actually. Maybe this exists somewhere but I have not found it. I'd rather not reverse engineer these assumptions from the code.

[bbatsov]

The documentation is here https://nrepl.org/nrepl/0.7.0/design/middleware.html#_pretty_printing

[thheller]

Thanks. Unfortunately that raises more questions than it answers.

:nrepl.middleware.print/print

a fully-qualified symbol naming a var whose function to use for printing. Defaults to the equivalent of clojure.core/pr.

This suggests that the handler isn't actually in charge of printing the result at all? The print middleware will instead take the regular :value and pprint it "later"? I mean who would otherwise call the fn specified in that qualified symbol? Is the eval handler supposed to load it? From CLJS I only get the already printed value back. Is this supposed to read that value back and then let it be pprinted later somewhere via CLJ? Or are those rules only for CLJ and CLJS just pprints itself and returns the value as a string with :printed-value like it does currently?

I'll go through the code I guess

[bbatsov]

@thheller Check the conversation on the PR I shared. We were discussing exactly the same things - ultimately we try to read the value as Clojure and apply the standard printing functions to it, but if we run into something Clojure can't process we fallback to using cljs.pprint. The approach is summarized here https://github.com/nrepl/piggieback#pretty-printing

[thheller]

I released 2.10.0 earlier which includes basically a complete new REPL implementation including nREPL. I tried to tackle this when rewriting it but couldn't figure out how this is supposed to work.

If I read the EDN string I get from the CLJS runtime and just send

{:value (edn/read-string edn-text)
 :nrepl.middleware.print/keys #{:value}
 :ns "cljs.user"}

as the message Cursive seems to have issues with this and ends up printing things incorrectly itself (eg. eval of :foo would lead to Cursive displaying foo as the result). I'm not entirely sure why this is and I can't tell if piggieback also has that problem in Cursive? I only tested this with basic values that should all round-trip correctly but only numbers, strings, bools do.

I don't have a working cider setup so I have no clue if cider would have gotten the correct result.

If someone cares enough about this and wants to dig in I'd be happy to help but otherwise I won't be spending much more time on this.

[bbatsov]

I'll have to check exactly what does print expect in the :value slot - EDN or string. I'll try to do this later today, but maybe @shen-tian or @ak-coram can beat me to it.

[bbatsov]

Sorry for the slow turnaround here. Very busy week... So, looking at Piggieback it seems you're doing the right thing:

{:value (when (some? result)
                                     (edn-reader/read-string
                                      {:default ->UnknownTaggedLiteral}
                                      result))
                            :nrepl.middleware.print/keys #{:value}
                            :ns (get @session #'ana/*cljs-ns*)}

I'm assuming your middleware doesn't depend on the print middleware and it doesn't get sequenced properly in the final middleware stack. Seems that's your descriptor, so I guess I'm right

shadow-cljs/src/main/shadow/cljs/devtools/server/nrepl04.clj

Line 9 in 92bb807

(middleware/set-descriptor!

I don't see print here as well

shadow-cljs/src/main/shadow/cljs/devtools/server/nrepl.clj

Line 84 in 92bb807

(defn make-middleware-stack [{:keys [cider] :as config}]

Btw, it's bit confusing there are several nREPL namespaces in the code (I guess a remnant of the transition from tools.nrepl to nrepl. Might be a good idea to fold them back together for simplicity's sake.

[bbatsov]

That's piggieback's descriptor as a reference https://github.com/nrepl/piggieback/blob/master/src/cider/piggieback.clj#L26

[thheller]

@bbatsov thanks, didn't realize this required a new middleware.

[bbatsov]

No problem. print sits on top of eval and controls the actual printing of results and output. This separation made it possible to address the historical issues of dealing with huge results that would bring to a halt the client and/or the server itself. It's not exactly new at this point - I wrote a bit about it a year ago https://metaredux.com/posts/2019/03/29/nrepl-0-6.html