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

Implement pprint support #18

Closed
bbatsov opened this issue Jun 3, 2020 · 6 comments · Fixed by #33
Closed

Implement pprint support #18

bbatsov opened this issue Jun 3, 2020 · 6 comments · Fixed by #33

Comments

@bbatsov
Copy link

bbatsov commented Jun 3, 2020

That's pretty useful feature that allows clients to specify the way results would be printed. I'm guessing that getting this to work with babashka shouldn't be very hard.

See https://nrepl.org/nrepl/0.7.0/design/middleware.html#_pretty_printing and https://github.com/nrepl/nrepl/blob/master/src/clojure/nrepl/middleware/print.clj for more details.

@kolharsam
Copy link
Contributor

@borkdude could I get started with this before #27 perhaps? Once I'm perhaps comfortable with this, I could then jump to #27. Hope that is okay?

@borkdude
Copy link
Contributor

@kolharsam Sounds good to me!

@borkdude
Copy link
Contributor

@kolharsam Note that babashka patches clojure.pprint since the default one bloats the binary with GraalVM a bit. The code for that is in babashka.impl.pprint. We just have to ensure that when this library is used with babashka, the babashka.impl.pprint namespace is loaded first. But we can see the effect of that in CI where binary size is printed.

@kolharsam
Copy link
Contributor

@borkdude just to be clear: while introducing support for pprint middleware for this, I need to keep in mind to load babashka.impl.pprint first and then build on top of that?
It seems like I'm reiterating your point but just wanted to make sure if I understood this correctly

@borkdude
Copy link
Contributor

@kolharsam We need to make sure that in babashka itself babashk.impl.pprint is loaded before babashka.nrepl.
I now checked main.clj and this is already the case:

   [babashka.impl.pprint :refer [pprint-namespace]]
   [babashka.impl.process :refer [process-namespace]]
   [babashka.impl.protocols :refer [protocols-namespace]]
   [babashka.impl.reify :refer [reify-opts]]
   [babashka.impl.repl :as repl]
   [babashka.impl.socket-repl :as socket-repl]
   [babashka.impl.test :as t]
   [babashka.impl.tools.cli :refer [tools-cli-namespace]]
   [babashka.nrepl.server :as nrepl-server]

So I think we're good.

@kolharsam kolharsam mentioned this issue Nov 29, 2020
borkdude pushed a commit that referenced this issue Feb 4, 2021
borkdude pushed a commit that referenced this issue Feb 4, 2021
* pprint support: Fix querying middleware

The pprint support erroneously used the value of the middleware as the
map to look into. Also, it seems that the type of the middleware is a
string, not a symbol.

This fixes sthis by swapping the argument order and changing the pretty
printer lookup up to use string keys.

* pprint support: wrap pprint to not use *out*

* pprint: Add test

* Test different pprint middleware settings
borkdude pushed a commit that referenced this issue Feb 4, 2021
borkdude pushed a commit that referenced this issue Feb 4, 2021
* pprint support: Fix querying middleware

The pprint support erroneously used the value of the middleware as the
map to look into. Also, it seems that the type of the middleware is a
string, not a symbol.

This fixes sthis by swapping the argument order and changing the pretty
printer lookup up to use string keys.

* pprint support: wrap pprint to not use *out*

* pprint: Add test

* Test different pprint middleware settings
@borkdude
Copy link
Contributor

borkdude commented Feb 5, 2021

All working now. Thanks @kolharsam, @Grazfather and @bbatsov!

@borkdude borkdude closed this as completed Feb 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants