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

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

Open
Macroz opened this issue Feb 12, 2019 · 26 comments
Open

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

Macroz opened this issue Feb 12, 2019 · 26 comments
Assignees

Comments

@Macroz
Copy link

Macroz commented Feb 12, 2019

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
Copy link
Contributor

aiba commented Feb 12, 2019

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

@cichli
Copy link
Member

cichli commented Feb 12, 2019

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
Copy link
Member

cichli commented Feb 12, 2019

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:

  • Use result buffer if it's alive, else cider-interactive-eval-output-destination
  • Use both result buffer (if it's alive) and cider-interactive-eval-output-destination

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

@cichli
Copy link
Member

cichli commented Feb 12, 2019

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

@cichli cichli self-assigned this Feb 12, 2019
@bbatsov
Copy link
Member

bbatsov commented Feb 13, 2019

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
Copy link
Member

bbatsov commented Feb 13, 2019

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
Copy link
Author

Macroz commented Feb 13, 2019

@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
Copy link

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
Copy link
Member

bbatsov commented Feb 13, 2019

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
Copy link
Author

Macroz commented Feb 13, 2019

I agree completely that consistency is good both in the implementation and user experience 👍.

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 😄

@aiba
Copy link
Contributor

aiba commented Feb 13, 2019

@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
Copy link
Author

Macroz commented Feb 13, 2019

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
Copy link
Member

bbatsov commented Feb 13, 2019

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
Copy link
Member

cichli commented Feb 13, 2019

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
Copy link
Member

cichli commented Feb 13, 2019

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
Copy link
Member

bbatsov commented Feb 13, 2019

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
Copy link
Member

bbatsov commented Feb 13, 2019

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
Copy link

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
Copy link

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
Copy link

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

@BrunoBonacci
Copy link

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

@bbatsov
Copy link
Member

bbatsov commented Mar 31, 2020

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

@BrunoBonacci
Copy link

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
Copy link

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

@BrunoBonacci
Copy link

@jvillste thanks for the link

blak3mill3r added a commit to blak3mill3r/cider that referenced this issue Jan 28, 2022
takes an optional `pretty` argument

stores the eval result in the kill ring

there was some discussion about this feature here
clojure-emacs#2580
blak3mill3r added a commit to blak3mill3r/cider that referenced this issue Jan 28, 2022
takes an optional `pretty` argument

stores the eval result in the kill ring

there was some discussion about this feature here
clojure-emacs#2580
@jvillste
Copy link

jvillste commented Feb 19, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants