Skip to content

Pretty view mode #335

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

Merged
merged 14 commits into from
Apr 18, 2025
Merged

Pretty view mode #335

merged 14 commits into from
Apr 18, 2025

Conversation

r0man
Copy link
Contributor

@r0man r0man commented Apr 11, 2025

Hello,

this PR adds the pretty printer from https://github.com/eerohele/pp (as suggested by @alexander-yakushev) and some of its tests, and a configuration option to toggle the pretty printing with the "P" key binding in the CIDER inspector.

The top level keys are rendered in the "inspector mode" as I understood it from a conversation on Slack.

The changes needed for the middleware can be found here: clojure-emacs/cider-nrepl#932
The changes for CIDER are here: clojure-emacs/cider#3813

The data structure shown on the following pictures was this:

{{:a 0
  :bb "000"
  :ccc []
  :d [{:a 0 :bb "000" :ccc []}
      {:a -1 :bb "111" :ccc [1]}
      {:a -2 :bb "222" :ccc [2 1]}
      {:a -3 :bb "333" :ccc [3 2 1]}
      {:a -4 :bb "444" :ccc [4 3 2 1]}]}
 {:a -1
  :bb "111"
  :ccc [1]
  :d [{:a 0 :bb "000" :ccc []}
      {:a -1 :bb "111" :ccc [1]}
      {:a -2 :bb "222" :ccc [2 1]}
      {:a -3 :bb "333" :ccc [3 2 1]}
      {:a -4 :bb "444" :ccc [4 3 2 1]}]}}

Without pretty printing enabled (as before):

map-normal-mode

With pretty printing enabled (in normal mode):

pprint-normal-mode

With pretty printing enabled (in object mode):

pprint-object-mode

Wdyt?

Open questions:

  • Pretty printing applies only to collections as of now. Are there other objects except from collections that should be pretty printed differently?
  • I removed many of the pp tests and now kept only some of them in the pp_test.clj file.
  • I noticed the "Value" in :object mode did also got pretty printed with this change, but the indentation was off, so I adjusted this as well.
  • Huge maps can look bad even in pretty mode. I tried it with one of our component system maps at work. I think the rendering could be improved if the content of a record (the {...} map after the record name) would be printed on a new line. Was the plan for the pretty printer to modify it for our needs?

TODOs:

  • I added 2 tests to orchard.print-test. They are passing in the REPL but fail when I run them with lein test orchard.print-test in Leiningen. For some reason they are printed differently in Leiningen. Any idea why that is? The atom (atom 1) for example is printed as "#atom[1 0x4f9ea948]" in the REPL, but like this #object[clojure.lang.Atom 0x3a718b52 {:status :ready, :val 1}] from Leiningen. I already checked that the various *print-XXX* vars are bound to the same value. It seems they are. What else could affect the printing?

Before submitting a PR make sure the following things have been done:

  • The commits are consistent with our contribution guidelines
  • You've added tests to cover your change(s)
  • All tests are passing
  • The new code is not generating reflection warnings
  • You've updated the changelog (if adding/changing user-visible functionality)

@r0man r0man force-pushed the pretty-view branch 2 times, most recently from 1e1d0a9 to b4ee290 Compare April 11, 2025 20:24
@alexander-yakushev
Copy link
Member

Thanks for putting in the work, @r0man! I left a few comments.

(render-value val (when mark-values?
{:value-role :map-value, :value-key key}))
(render-ln)))
(let [rendered-key (rendered-element-str (last (:rendered (render-map-key ins key))))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still not quite what I meant:) You can print the key with print-string first. That gives you a string that you can check if it's long or has newlines and you don't have to dig it out of :rendered list and parse it. And once you have a printed string, you can pass it to render-value via :display-value.

@alexander-yakushev
Copy link
Member

Another thing: even though it is a config option now, I think it will still be beneficial to have a toggle-pretty-print function similar to toggle-view-mode, so that we can then also have a toggle op on cider-nrepl and thus on CIDER we can have a toggle command that doesn't need to know on Elisp side whether the pretty-printing is currently enabled.

@alexander-yakushev
Copy link
Member

Whoops, sorry for confusing you! I forgot that toggle-view-mode is implemented completely on cider-nrepl side. Right, it is enough to have :pretty-print as a configuration option, and then implement toggle-pretty-print in cider-nrepl since it has complete access to inspector state anyway. Sorry again.

@r0man
Copy link
Contributor Author

r0man commented Apr 18, 2025

@alexander-yakushev No worries, I think what you describe in your last comment was my latest plan. Was your comment about adding the inspect-toggle-pretty-print NREPL op AND adding a toggle-pretty-print function to the orchard inspector, which the middleware would call? I thought about doing this, but have not worked on it yet. Given that we usually just delegate to the orchard inspector, why would you not do it in this case?

@alexander-yakushev
Copy link
Member

alexander-yakushev commented Apr 18, 2025

I think it is perfect as it is. Let's merge!

adding a toggle-pretty-print function to the orchard inspector, which the middleware would call?

Yeah, I suggested this and then understood it is not necessary.

@alexander-yakushev alexander-yakushev merged commit a0f1dcf into clojure-emacs:master Apr 18, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants