search

tpope / vim-fireplace

fireplace.vim: Clojure REPL support
https://www.vim.org/scripts/script.php?script_id=4978
1.75k stars 139 forks source link

Add pprint to cpp [#262] #301

Closed SevereOverfl0w closed 6 years ago

SevereOverfl0w commented 7 years ago

The default is the same as what CIDER.el uses, and seems sensible. I got significantly better results using fipp than clojure.pprint

This could be extended to other evaluations quite easily too, I think :Eval would benefit, but I only matched the suggestion you made in #262

tpope commented 7 years ago

Surely g:fireplace_pprint_max_right_margin would make more sense as &columns (or maybe &columns - 1), no?

SevereOverfl0w commented 7 years ago

I matched the default width of clojure.pprint/pprint. I (personally) feel like a width of &columns on my screen might be a little wide for reading, but I'm not sure if fipp/pretty printers will collapse in a sane way despite that.

Happy to make the change if you're certain though. Is a special value like -1 or 'calculate-columns' the way to do it?

SevereOverfl0w commented 7 years ago

Comparison of &columns and 72 on a reflection of java.io.File.

maim-2017-07-09-08 07 00 maim-2017-07-09-08 07 36

My read width expands far past the &columns. Having said that, I've pushed a patch which makes the change as requested.

tpope commented 7 years ago

Let's name the option g:fireplace_print_right_margin, to more closely mirror the API. Use get(g:, 'fireplace_print_right_margin', &columns) with no global default to achieve the correct behavior with no need for a sentinel value like "&columns".

Instead of hard coding a list of pprint options in the nrepl adapter, try something like this:

for [k, v] in items(options)
  let msg[tr(k, '_', '-')] = v
endfor

This should enable cleaning up some of the other msg mutating code to boot.

SevereOverfl0w commented 7 years ago

I've made the changes requested. nrepl adapter is much nicer.

tpope commented 7 years ago

Haven't fully audited this but I spy a spurious echo.

SevereOverfl0w commented 7 years ago

Oops, thought I'd pushed over that. Gone.

tpope commented 7 years ago

I can't get this to work. It's using the load-file middleware, which appears to not support pretty print?

SevereOverfl0w commented 7 years ago

Load file delegates to eval, so it should work. Obligatory, do you have cider 0.15 loaded?

On 26 Jul 2017 05:59, "Tim Pope" notifications@github.com wrote:

I can't get this to work. It's using the load-file middleware, which appears to not support pretty print?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tpope/vim-fireplace/pull/301#issuecomment-317947864, or mute the thread https://github.com/notifications/unsubscribe-auth/AF2h2i7rEK6CbiguQ0cYpyne6uhct_jLks5sRse-gaJpZM4ORrQd .

SevereOverfl0w commented 7 years ago

So, this is strange. It didn't work on my first evaluation, so I added echo msg to the nrepl adapter, and restarted vim, at which point it did work.

I've now deleted the echo msg, and it still works.

I have no idea why, have you ever seen something like this?

SevereOverfl0w commented 7 years ago

This works/breaks across restarts of the JVM, I'm guessing that CIDER may need to be a little more forceful about being in front of load-file as well as eval.

SevereOverfl0w commented 7 years ago

CIDER has accepted my PR fixing the non-deterministic pretty print. It will be in 0.15.1, or you can build from master.

SevereOverfl0w commented 6 years ago

0.15.1 is now released, so this PR can be tested.

SevereOverfl0w commented 6 years ago

Looking at :Eval, this seems trivial to add there. Any reason to not include that in this PR?

SevereOverfl0w commented 6 years ago

I've pushed a commit which adds support for :Eval, I've been using this locally for a while and not seen any problems.

Olical commented 6 years ago

I'm currently using this on pretty large data structures, very happy with the results. Would be awesome to have this good work in master :slightly_smiling_face:

devth commented 6 years ago

I just pulled latest but I'm not seeing pprint'd structures with cpp or Eval. Do I need to activate it? Not seeing anything in the source 🤔 Using cider 0.16.0.

Olical commented 6 years ago

I'm using the branch and fork mentioned in this PR, it's not merged yet. Just voicing my support and testing for it :)

On Fri, 13 Apr 2018, 00:53 Trevor Hartman, notifications@github.com wrote:

I just pulled latest but I'm not seeing pprint'd structures with cpp or Eval. Do I need to activate it? Not seeing anything in the source 🤔 Using cider 0.16.0.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/tpope/vim-fireplace/pull/301#issuecomment-380978438, or mute the thread https://github.com/notifications/unsubscribe-auth/AATPXVUnMs452S5-NA_qtwQwQfUtFtpOks5tn-jigaJpZM4ORrQd .

devth commented 6 years ago

@Olical it's merged!

I got it working mysteriously after restarting vim several times and messing around with my init.vim. /shrug

This is amazing. 💯