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

Use streamed printing of values #336

Closed
bbatsov opened this issue May 25, 2019 · 9 comments
Closed

Use streamed printing of values #336

bbatsov opened this issue May 25, 2019 · 9 comments

Comments

@bbatsov
Copy link

@bbatsov bbatsov commented May 25, 2019

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

This comment has been minimized.

Copy link
Owner

@tpope tpope commented May 26, 2019

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

This comment has been minimized.

Copy link
Author

@bbatsov bbatsov commented May 26, 2019

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

This comment has been minimized.

Copy link
Owner

@tpope tpope commented May 26, 2019

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

This comment has been minimized.

Copy link
Author

@bbatsov bbatsov commented May 26, 2019

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

This comment has been minimized.

Copy link
Owner

@tpope tpope commented May 26, 2019

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

This comment has been minimized.

Copy link
Owner

@tpope tpope commented May 27, 2019

Okay I finally booted up Emacs:

user> 1 2
12
user> 

At least I'm not crazy.

@bbatsov

This comment has been minimized.

Copy link
Author

@bbatsov bbatsov commented May 27, 2019

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 - nrepl/piggieback#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

This comment has been minimized.

Copy link
Author

@bbatsov bbatsov commented May 27, 2019

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

This comment has been minimized.

Copy link
Owner

@tpope tpope commented May 27, 2019

More precisely, the behavior appears to be:

  • If the print middleware is active, a separate message containing the namespace is sent after each whole value.
  • If it's not, every message contains both a whole value and a namespace.

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.

@tpope tpope closed this in cef303b May 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.