tpope / vim-fireplace

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

Use streamed printing of values #336

Closed bbatsov closed 5 years ago

bbatsov commented 5 years ago

Value streaming is a new feature in nREPL 0.6 and it allows the clients to receive results in chunks and to interrupt the evaluations resulting in something huge (this also extends to output, btw). I've written about it a bit here. The official docs are here. Switching to streaming printing of everything is a great quality-of-life improvement and requires minimal changes to clients.

tpope commented 5 years ago

So if I understand correctly, I effectively need to concatenate all :value values to one big string. But if the middleware isn't available or fails, then concatenating the :value values of evaluating 1 2 (with multiple returned values) will result in 12. Is there a way around this?

bbatsov commented 5 years ago

So if I understand correctly, I effectively need to concatenate all :value values to one big string.

Generally you don't need to concatenate them. In most cases it should be fine to just start printing the chunks (that's what I do in CIDER). That also ensure that the users start seeing something as quickly as possible.

But if the middleware isn't available or fails, then concatenating the :value values of evaluating 1 2 (with multiple returned values) will result in 12. Is there a way around this?

This particular middleware never fails. If it fails you won't get a string representation of a value in the first place. It's also always available, provided you're on nREPL 0.6, which pretty much is these days (it's the version bundled with recent Lein and Boots). Basically you used to get one value message, now you can get multiple. Same with out. I guess if you simply print you don't really care if you get got multiple responses or not. And if you're doing some internal eval that won't be rendered to the use you can just avoid passing the stream? flag and you'll get the whole result.

tpope commented 5 years ago

The middleware can certainly "fail" if you pass it the name of a print function that doesn't exist, for example. Right now I'm in a position where if Cider is installed, I'll get back "1\n" "2\n", but if it's not, I get back "1" "2". So if I "just start printing", I'm back to the 12 problem.

bbatsov commented 5 years ago

Ah, I finally got what you mean. Sorry! Well, you'd have the same problem without streamed printing as well - pretty printing and streamed printing are not really related. I think the term pretty-printing right now is a misnomer as the new functionality is just a generic printing facility for any response key. I guess we can safely assume that if users start poking the printer and its configuration they should know what they are doing, right? For your particular case the values won't be 1 and 2 - they'll be 1 and 2 or 1 and 2, so the resulting printed output would be just fine. (I'm assuming you're talking about those as the potential elements of some bigger list or whatever)

tpope commented 5 years ago

Oh I'm not talking about a list, I'm talking about literally sending 2 different expressions on a single line. It's not a particularly compelling use case - it probably happens more often accidentally than deliberately - but the accidental case still means you get confusingly corrupted output.

tpope commented 5 years ago

Okay I finally booted up Emacs:

user> 1 2
12
user> 

At least I'm not crazy.

bbatsov commented 5 years ago

Definitely! That's an oversight our part (nREPL's team). Evaluating multiple expressions is so uncommon (and not supported by Piggieback at all for performance reasons - https://github.com/nrepl/piggieback/pull/98), that we forgot about it. I'll open an upstream ticket.

I wouldn't worry about this use-case too much, though, as you're the very first person to spot this problem.

bbatsov commented 5 years ago

Btw, when debugging this I've noticed that there's actually one extra message after each complete value:

(-->
  id                             "8"
  op                             "eval"
  session                        "02030b7f-421a-40b4-a01e-eee0db7954bb"
  time-stamp                     "2019-05-27 08:35:44.459611000"
  code                           "1 2"
  column                         18
  content-type                   "true"
  file                           "*cider-repl projects/hard-cider:localhost:64491(clj)*"
  line                           43
  nrepl.middleware.print/options (dict ...)
  nrepl.middleware.print/print   "cider.nrepl.pprint/pprint"
  nrepl.middleware.print/quota   1048576
  nrepl.middleware.print/stream? "1"
  ns                             "hard-cider.core"
)
(<--
  id         "8"
  session    "02030b7f-421a-40b4-a01e-eee0db7954bb"
  time-stamp "2019-05-27 08:35:44.722417000"
  value      "1"
)
(<--
  id         "8"
  session    "02030b7f-421a-40b4-a01e-eee0db7954bb"
  time-stamp "2019-05-27 08:35:44.722966000"
  ns         "hard-cider.core"
)
(<--
  id         "8"
  session    "02030b7f-421a-40b4-a01e-eee0db7954bb"
  time-stamp "2019-05-27 08:35:44.723155000"
  value      "2"
)
(<--
  id         "8"
  session    "02030b7f-421a-40b4-a01e-eee0db7954bb"
  time-stamp "2019-05-27 08:35:44.723492000"
  ns         "hard-cider.core"
)
(<--
  id         "8"
  session    "02030b7f-421a-40b4-a01e-eee0db7954bb"
  time-stamp "2019-05-27 08:35:44.723703000"
  status     ("done")
)

I know that's no ideal, but it can be used as a way to differentiate the multiple results.

tpope commented 5 years ago

More precisely, the behavior appears to be:

It's a little convoluted, but certainly seems possible to disambiguate.

Tag me in on whatever issue ends up getting created, I wanna see where this goes.