Limit the maximum string size to be displayed in echo area

[alexander-yakushev]

Although there is still work to be done (and thoughts to be thought) in cider-nrepl or nREPL regarding how much we want to print, it makes sense to solve such an issue on CIDER side too. Currently, if cider-use-overlays is not t, the standard cider-interactive-eval-handler will try to display the whole printed result in the minibuffer (echo area). For some reason, pushing huge strings through the echo area is much slower than printing it to the REPL. E.g., doing (println (range 200000)) is almost instant while evaluating (range 200000) in the source buffer takes ~3 seconds on my machine to display the truncated result. Emacs would obviously still truncate the echo area output but not before printing the whole response to *Messages*.

This PR proposes to truncate the long output ahead of time. We can also add the similar message that we currently show in overlays, something like "Result truncated. Type M-x cider-inspect-last-result to inspect it. if it is deemed necessary.

Example of how the truncated result would look like:

[alexander-yakushev]

I guess this intersects with #3635 (only spotted it now).

While I think that PR solves a more general problem, it may make sense to restrict it here as well. Dunno, let's discuss.

[vemv]

Seems a good improvement - thanks!

[vemv]

Two nits:

• seems better to reuse the let* rather than to setq - it's less imperative
• string-trim-right seems slightly better performed after the trimming?

[vemv]

cider--make-result-overlay font-value is using the non-trimmed value

[alexander-yakushev]

That's because cider--make-result-overlay performs its own trimming, so I guess it makes sense to pass the full result to it. In case, in the future, it decides to do something else with the result, so we don't obstruct that here.

[alexander-yakushev]

Ah, you meant trimming as in string-trim.

[vemv]

No, I didn't mean string-trim, but length trim

That's because cider--make-result-overlay performs its own trimming

Alright then 👍

[alexander-yakushev]

I've looked through #3635 and now I think that my PR is unnecessary. While with that PR, the echo area would still receive more than it could handle (e.g., 10k characters instead of ~1500 with this PR), the difference in perceived latency is not noticeable. On the plus side, we won't be doing similar work in two places.

If we can fast forward #3635 – that will be great.

[vemv]

While I think that PR solves a more general problem, it may make sense to restrict it here as well. Dunno, let's discuss.

I don't mind about having both in place - this can be a last-resource measure.

A drawback being that users (/ maintainers) have to understand/tweak more stuff (e.g. let's imagine that each of the solutions has a different defcustom, or in any case, a different algo).

Maybe for the sake of containing complexity (i.e. favoring existing tech, namely the "print quotas"), let's go for #3635 . I can polish it later this week or if it feels like a blocker, I'd be OK with you taking over it.

[vemv]

If we can fast forward #3635 – that will be great.

You beat me to it ^^

as mentioned, I can go back to it soon, else you can take over.

[alexander-yakushev]

Actually, let me reopen.

I just understood that because an overflowing message in the echo area truncates the beginning:

then, after #3635 is applied (which truncates the end), an overflowing message will show the middle of printed value, which is going to be quite ugly and probably quite confusing. Currently, at least, the user gets to see the end of the value and understand the return type (here – a list), but when shown the middle of the message they won't even get that.

Also, arguably the beginning of the value is more useful then the end of it.

So, I think, we'll still need this PR, and perhaps even more so after #3635.

[vemv]

If a small enough quota was requested for ops which output goes to the echo area, why would there be truncating in the beginning?

[alexander-yakushev]

That implies that different quota is used when the output is going to the minibuffer vs the REPL vs somewhere else. From #3635 I gathered that it isn't going to be the case.

Besides, this goes against how cider--display-interactive-eval-result is implemented (at least, currently). The function receives a single result, and then sends it to two destinations (if cider-use-overlays = 'both). I guess, the echo area limit is still larger than the overlay limit (the overlay one is now at 3*frame-width), but making the presentation depend on that feels kinda flaky to me.

[alexander-yakushev]

Addressed the comments.

A drawback being that users (/ maintainers) have to understand/tweak more stuff (e.g. let's imagine that each of the solutions has a different defcustom, or in any case, a different algo).

I don't think we would need a defcustom for this one. Here, we are lucky to get "help" from the minibuffer implementation by being kinda "broken" currently since it can't go over (max-mini-window-lines). So, in my opinion, any solution that we provide is better than status quo, and hence, we aren't going to break any current UX for people to miss.

[vemv]

That implies that different quota is used when the output is going to the minibuffer vs the REPL vs somewhere else. From #3635 I gathered that it isn't the case.

You're partially right, but in that PR some parts were intentionally unchanged to not tighten the quota.

So indeed the 'overlay' and 'echo area' limits would be coupled.

Honestly I believe that it would be a very minor problem / edge case, because:

• most values are small
• for big values, most people won't carefully read every character in the echo area if they nearly fill their screen
  • in the inline overlay we can pretty-print things, with font-locking
  • there's defcustom cider-auto-inspect-after-eval as well.

So my thinking is that the echo area can help as a quick confirmation that evaluation succeeded - its rough value.

Because it's 'transient', it only has relative value as it can easily go away with keypresses or other events.

So, tldr I don't exactly oppose this PR but it only seems useful for making very specific things like (range 20000) nicer. For most exprs there isn't a huge difference or a problem to begin with.

I'd prefer to keep things simple and not add further moving pieces, since they only address edge cases in the less favored of the available UIs.

[alexander-yakushev]

I agree with you that the problem is not a huge one. But I'd still prefer to see the beginning of the value in the minibuffer if the value is too large. At least, it will be consistent with how we trim values everywhere else.

(range 20000) is an example of the value that we will only show the middle of after #3636 is merged. But for any value over (max-mini-window-lines), we'll still show the tail instead of the head, and seeing the head at a quick glance, in my opinion, gives much better information about the structure of the response, even if the tail is truncated.

[vemv]

No, I didn't mean string-trim, but length trim

That's because cider--make-result-overlay performs its own trimming

Alright then 👍

[alexander-yakushev]

since they only address edge cases in the less favored of the available UIs.

To your point about the UI being less favored – it is true that the default for cider-use-overlays is 'both, however:

1. Both means we still print to minibuffer by default, so it is still visible to the user, even if the same value is also shown as an overlay. So we can assume that most users see the minibuffer output.
2. While I may be in the minority of users who disable overlays and only print the result to the minibuffer, the minibuffer is still the primary communication vessel in Emacs, so I would avoid writing it off as redundant or obsolete.

So, I think, if we can improve the UX of it with small costs, then it is reasonable to do so.

[alexander-yakushev]

Thanks for the review!

[vemv]

Cheers.

Frankly I'd review the default of cider-use-overlays at some point - overlays have gotten better and better over the last year, so showing duplicated info (and that can often look worse for the echo area part) makes little sense.

[bbatsov]

Seems we need to print font-value instead of value, otherwise we'd lose the font-locking (if it was done).

[bbatsov]

Probably here font-locked-value would have been a better name, although depending on the config it might not be font-locked. Naming is hard...