Add inspect-print-current-value op

[r0man]

This PR adds the inspect-print-current-value op. It prints the current value of the inspector with the user provided ::print/print-fn or a default one.

It is used by CIDER in the cider-inspector-print-current-value command, bound to "P" in the inspector, to print it's current value. clojure-emacs/cider#3810

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 mostly passing (there seem to be some unrelated flaky tests)
• You've updated the CHANGELOG
• Middleware documentation is up to date

[alexander-yakushev]

I can't say I immediately understand the usecase, to be honest.

1. Inspector is already a "better pretty-print", so there is less value in printing the value to stdout if you are looking at it in the inspector.
2. We'll have a separate pretty-printing view that can be switched on inside the inspector.
3. After pretty-printing the value to stdout, if the user only had the inspector window open, they will have to close it or explicitly navigate to the REPL buffer to see what's had been pprinted. Can this be convenient?

[r0man]

@alexander-yakushev Have you seen the screenshot and my explanation here: clojure-emacs/cider#3810 ?

It is not printing to stdout, it opens the value in a popup buffer, like cider-pprint-eval-last-sexp. I also mentioned there why I sometimes prefer to look at the pretty printed version of it, vs the one we show in the inspector today.

That said, let's see where we land with the pretty mode before continuing with this.

[r0man]

The jdk8 test is failing because of this:

ERROR in (continue-non-stop) (debug_integration_test.clj:293)
Uncaught exception, not in assertion.
expected: nil
  actual: clojure.lang.ExceptionInfo: Unexpected timeout waiting for nREPL response. Expected message: {:status ["done"]}

I think it's a flaky test, that I have seen failing before.

[bbatsov]

One question on my mind is whether we need a dedicate op to print the inspector value. I was thinking it might also makes sense to have an op that fetches the current inspector value and for it to be printed separately. I guess the current suggestion would be more efficient, but it seems oddly specific to me.

[r0man]

Hi @bbatsov, I think we need one. Initially I had an op that just returned the rendered value as a string. But I changed it because it caused slowness on large data structures and NREPL timeouts.

[alexander-yakushev]

@bbatsov I understand what you are saying. But what's "fetching the current inspector value"? You can't "return" a value from nrepl server to the client, it has to be printed into a string anyway.

[bbatsov]

Yeah, yeah. I'm aware of this - that's why I said that probably it wouldn't be particularly efficient, as you'd need to read the printed string and then pretty print the underlying value again. I was just wondering if we can't come up with something more generic for "printing values stored in X, Y, Z", but I haven't given this much thought. I don't have any serious concerns about the current proposal - just thinking in writing. 😀

[r0man]

@alexander-yakushev I moved this out of with-safe-transport because I saw an exception in the *nrepl-messages-buffer*, which I think was caused by the new op sending mutiple lines vs the ops in with-safe-transport returing a response value.

I didn't notice this exception before, because from the user's perspective both seemed to work.

With this change I don't see the exeception anymore. Can you double check that this ok?

[alexander-yakushev]

Yeah, I think it makes sense. This op is indeed different in this regard. It's fine to leave this as is, just leave a comment why it stands separately.

[bbatsov]

@alexander-yakushev Should we prefix this with cider/ from the beginning or leave this to for another day? (e.g. when we add aliases for the all the existing ops)

[alexander-yakushev]

I don't mind; might as well add the prefix now indeed. Would you do it, @r0man?

[r0man]

I would keep it consistent and do it once we decide to rename all. But I can also do this now (we are talking about this op only right?) if you prefer.

[bbatsov]

Okay, let's do this later.