clojure-emacs / cider

The Clojure Interactive Development Environment that Rocks for Emacs
https://cider.mx
GNU General Public License v3.0
3.55k stars 645 forks source link

Changed handling of stdout for cider-pprint-eval-last-sexp #2580

Open Macroz opened 5 years ago

Macroz commented 5 years ago

Is nobody troubled by the change that cider-pprint-eval-last-sexp does not direct stdout to the same buffer as the eval result? Or there doesn't seem to be an option to enable this old behavior.

I.e. try to eval this with C-c C-p and see that the print is not shown in the buffer. (list (prn 1) 2)

I use C-c C-p constantly and never look at the REPL or some separate stdout buffer. I used to be able to add debug prints here and there and keep track of the results with the debug prints.

Expected behavior

A new buffer opens with 1 and (nil 2) shown.

Actual behavior

A new buffer opens with only (nil 2) shown. The stdout 1 goes to REPL buffer.

Steps to reproduce the problem

cider-pprint-eval-last-sexp this code (list (prn 1) 2)

Environment & Version information

CIDER 0.21.0snapshot 20190125.1339

aiba commented 5 years ago

Yes, me too. I much preferred having stdout go to *cider-result*. I would love to have that functionality back.

cichli commented 5 years ago

I agree. If you rely on C-c C-p a lot it's extremely convenient to have the output from *out* and *err* in the popup as well. Having the option to redirect it to a different buffer is useful sometimes (for example if it ends up interleaved with the printed result due to laziness) but in most cases that isn't a problem, so I'm inclined to just revert to the old behaviour for now.

There are smarter ways to deal with the interleaving problem, which weren't possible with cider.nrepl.middleware.pprint, but are now with the new version of nREPL.

cichli commented 5 years ago

The redirecting is also (possibly) useful if you kill the result buffer. Maybe some future body or thread is still printing to *out*, even long after the evaluation returned. What should we do with that output? Two possible strategies:

This would only apply to *out*/*err*. Killing the buffer should always stop printing the value.

cichli commented 5 years ago

Just for my own reference, relevant change is 34783f5551c656667c20217171179769a3bc6bc8.

bbatsov commented 5 years ago

I understand the concerns with the change, but that command is supposed to simply pretty-print a result and do nothing more (the behavior should be consistent with regular evaluation). The old behavior is something I consider a bug. If someone wants a buffer that captures everything related to an evaluation then perhaps this should be a different command.

bbatsov commented 5 years ago

One more thing - the only reason why pretty-printed results are shown differently (namely in a buffer instead of inline/in the mini buffer) is that those results are often several lines long and simply don’t look well otherwise. Everything that wasn’t related to printing a result in the command was accidental behavior, therefore my suggestion that perhaps a different command is needed. Anyways, I’m certain @cichli will figure some nice solution to this problem.

Macroz commented 5 years ago

@bbatsov I don't quite get your point. Why should the printing here be any different to what happens if you eval the expression in the REPL? Don't you get the printing and results (possibly intermingled) there too? You don't have to go look for them. It's slightly different if you insist on evaling the result into a comment etc.

To me the difference is that instead of having to go to REPL you get each evaluation with the potential side effects that happen during it to its own convenient buffer.

jvillste commented 5 years ago

I have also got used to having stdout along side with the result. I think it would be nice leave the default behaviour as is and to introduce a customisable parameter that hides out and err from the buffer.

bbatsov commented 5 years ago

I don’t like this idea, as it introduces an incostency with the rest of the eval commands. All eval commands are supposed to display the output either in the REPL or in a dedicated output buffer (depending on the configuration). This command wasn’t doing this, due to a historical accident, and that’s the point I was trying to make - commands that don’t behaving like consistently introduce mental overload and confusion.

Your arguments would make sense if the command was intended to do anything else than printing results, but it’s not, that’s I said that if we need something that displays results and output together in a buffer this should be either a different command or a different configuration to existing eval commands.

I know this might sound strange and frustrating to you, but I think that it’s more important to build a uniform set of functionality than to try to preserve accidental behavior, just because they were useful in some contexts. The missing functionality should be added in a way that it doesn’t compromise the overall uniformity of everything.

Macroz commented 5 years ago

I agree completely that consistency is good both in the implementation and user experience :+1:.

There are currently a ton of similar but slightly differently named eval functions and the confusion is apparent. I would have gladly built myself the functionality if the different bits were a bit more apparent and composable. So I'm happy to wait for your improved implementation.

Using the REPL as it is just feels like going back to the stone age :smile:

aiba commented 5 years ago

@bbatsov I appreciate the design principle that would want to keep *cider-result* as just the result, not any output side effects. For example if you wanted to write elisp code that evaluated an expression and then took the output from the *cider-result* buffer, you definitely wouldn't want stdout strings in there.

Still, I can attest to the productivity value in being able to evaluate an expression and see both the stdout side-effects as well as the pretty-printed result in one buffer. Unlike @Macroz I don't have an issue with a ton of slightly differently named eval functions. I just read through cider-eval.el and I think it's pretty clear what does what.

So is this now just a matter of coming up with a new eval function, perhaps cider-pprint-eval-last-sexp-with-stdout?

Macroz commented 5 years ago

The combinatorial explosion of similar functions like cider-pprint-eval-last-sexp-with-stdout would be just what I'd personally want to avoid.

bbatsov commented 5 years ago

Yeah, absolutely. That’s why probably it’d be best to come up with some good combination of configuration + prefix params for the relevant commands. I guess we’ll have to think a bit about the best course of action.

I’ve long been unhappy about the current state of the eval commands and even though things have been slowly becoming more consistent and predictable over time, I do believe at some point we’ll need to tackle this head on and try to solve the root problems. Frankly, even I can’t tell you what each command does simply by looking at its name. 😆

cichli commented 5 years ago

I wonder if we could use the REPL buffer instead of a dedicated buffer for pretty printed results. The output will be sent to cider-interactive-eval-output-destination, same as any other REPL evaluation.

C-c C-p is more convenient than the REPL directly since: 1) you don't have to insert the form into the REPL, and 2) you don't have to set the REPL namespace. We could make sure that happens (or appears to happen) automatically. There is the question of what to do if there's already partial input after the REPL prompt (maybe just save and kill the entire prompt and input, then put it back after the evaluation is done).

One thing I like about the dedicated buffer is that it's easy to select the printed value and copy it into another buffer or application compared to at the REPL. For convenience we could also add a keybinding to the REPL to copy the last printed value to the kill ring.

cichli commented 5 years ago

Frankly, even I can’t tell you what each command does simply by looking at its name.

I also find the keybindings somewhat confusing.

bbatsov commented 5 years ago

Legacy. :-)

In the early days I didn’t think much about the future and the consequences of some rash decisions. Lesson learned. :D The dedicated eval keymap was an attempt to tackle this to some extent, but there’s obviously a lot of room for improvement. I also wonder how many people use it in practice (or know that it even exists).

bbatsov commented 5 years ago

I wonder if we could use the REPL buffer instead of a dedicated buffer for pretty printed results. The output will be sent to cider-interactive-eval-output-destination, same as any other REPL evaluation.

We might benefit from adding something like eval-result-destination, although I really don’t know how we can print well a multi-line results in the minibuffer or inline. Probably some prefix args can affect the default selection.

One thing I like about the dedicated buffer is that it's easy to select the printed value and copy it into another buffer or application compared to at the REPL. For convenience we could also add a keybinding to the REPL to copy the last printed value to the kill ring.

Yeah, that’s a good idea. Probably we can have such a key binding in result buffers as well (if we decide we need them).

jvillste commented 5 years ago

One thing I like in the separate result buffer is that it is clear where the output of the last evaluation starts. If I print to the REPL I have to print empty lines to mark the beginning.

On the other hand it is some times useful to have the whole history accessible in the REPL buffer. Maybe it should be possible to print the results to both in the REPL buffer and a separate buffer.

BrunoBonacci commented 5 years ago

I use extensively this feature as well, pretty much my only way to evaluate, I understand that the previous behavior wasn't supposed to work that way, but I would appreciate if you would consider adding an option to output the *out* and *err* back into the *cider-result* buffer.

sathyavijayan commented 5 years ago

I use and love this feature too - it will be great to add an option to keep the current behaviour !

BrunoBonacci commented 4 years ago

@bbatsov is there any chance to get the *out* and *err* back into the *cider-result* buffer any time soon?

bbatsov commented 4 years ago

Can't make any promises. I've got many tasks I'd like to tackle and very little time right now.

BrunoBonacci commented 4 years ago

I feel your pain. I'm not too competent in emacs-lisp but I can help you on the Clojure-side if there is anything pressing.

jvillste commented 4 years ago

I've been using this branch for a while to get this working: https://github.com/jvillste/cider/tree/std-streams-to-popup

BrunoBonacci commented 4 years ago

@jvillste thanks for the link

jvillste commented 8 months ago

I used a forked cider for many years to get this working but now there were too many merge conflicts when I tried to update cider and realized that I can easily have stdout and stderr in cider result buffer without touching cider implementation. Here are bunch of evaluation commands that do the trick: https://gist.github.com/jvillste/b7753bcfee752a05aeff20683d0bd6fd

I think marking a sexp or a function to be avaluated later and then evaluating it after each code change so that stdout and the result show up in the result buffer is really nice way of debugging and developing. I learned that form @Macroz and those commands are originally from him.