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.

Sign up for GitHub

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

Jump to bottom

Eval into register, kill last eval result #3162

Merged
merged 8 commits into from Apr 27, 2022
Merged

Eval into register, kill last eval result #3162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
583167e
Save eval results into custom register
yuhan0 Feb 10, 2022
37f8aa2
Add command for saving last eval to kill ring
yuhan0 Feb 22, 2022
03753ca
Add keybinding
yuhan0 Feb 27, 2022
55dffd1
Fix typo in pr-request-map
yuhan0 Mar 15, 2022
1351a8e
Handle streaming of results in eval handlers
yuhan0 Mar 15, 2022
b2ceae2
Remove eval-register from printing handlers
yuhan0 Apr 27, 2022
bf5f74d
Update changelog
yuhan0 Apr 27, 2022
ecc8de8
Add docs for eval registers
yuhan0 Apr 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -5,6 +5,9 @@
## New features

* Add new interactive command `cider-inspire-me`. It does what you'd expect.
* [#3162](https://github.com/clojure-emacs/cider/pull/3162): Save eval results into kill ring and registers.
* Add new customization variable `cider-eval-register` to automatically store the last interactive eval result into the specified register.
* Add interactive command `cider-kill-last-result` to manually save the last eval result into kill ring.

### Changes

Expand Down
4 changes: 2 additions & 2 deletions cider-client.el
View file
Open in desktop
Expand Up @@ -397,8 +397,8 @@ is included in the request if non-nil."
(seq-mapcat #'identity)
(apply #'nrepl-dict))))
(map-merge 'list
`(("nrepl.middleware.print/print" "cider.nrepl.pprint/pr"
"nrepl.middleware.print/stream?" nil))
`(("nrepl.middleware.print/print" "cider.nrepl.pprint/pr")
("nrepl.middleware.print/stream?" nil))
(unless (nrepl-dict-empty-p print-options)
`(("nrepl.middleware.print/options" ,print-options)))
(when cider-print-quota
Expand Down
106 changes: 75 additions & 31 deletions cider-eval.el
View file
Open in desktop
Expand Up @@ -167,6 +167,15 @@ If t, save the file without confirmation."
:group 'cider
:package-version '(cider . "0.16.0"))

(defcustom cider-eval-register ?e
"The text register assigned to the most recent evaluation result.
When non-nil, the return value of all CIDER eval commands are
automatically written into this register."
:type '(choice character
(const nil))
:group 'cider
:package-version '(cider . "1.4.0"))


;;; Utilities

Expand Down Expand Up @@ -647,16 +656,21 @@ evaluation command. Honor `cider-auto-jump-to-error'."
(defun cider-insert-eval-handler (&optional buffer)
"Make an nREPL evaluation handler for the BUFFER.
The handler simply inserts the result value in BUFFER."
(let ((eval-buffer (current-buffer)))
(let ((eval-buffer (current-buffer))
(res ""))
(nrepl-make-response-handler (or buffer eval-buffer)
(lambda (_buffer value)
(with-current-buffer buffer
(insert value)))
(insert value))
(when cider-eval-register
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this break for bigger results that are streamed in several chunks? Seems to you your register will only retain the last chunk.

Copy link
Contributor Author

@yuhan0 yuhan0 Mar 15, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late response - I spent a while trying to reproduce and understand this chunking behaviour.
It seems that by default CIDER only enables the stream chunking in certain pretty printing commands, and the handlers for other commands operate on the assumption that the result is not chunked.

See cider-eval-defun-to-comment ->
cider-eval-print-with-comment-handler (does not handle chunks)
with option map (cider--nrepl-pr-request-map) (disables streaming)

whereas cider-pprint-eval-defun-to-comment ->
cider-eval-pprint-with-multiline-comment-handler (accumulates chunks)
with option map (cider--nrepl-print-request-map fill-column) (enables streaming)

But one could conceivably call one with the other, with broken results:

(cider-interactive-eval "(range 50)"
                        (cider-eval-print-with-comment-handler
                         (current-buffer) (point-max-marker) cider-comment-prefix)
                        nil
                        '(("nrepl.middleware.print/stream?" "1")
                          ("nrepl.middleware.print/buffer-size" 20)))
;; => )
;; => 43 44 45 46 47 48 49
;; => 6 37 38 39 40 41 42 
;; =>  30 31 32 33 34 35 3
;; => 23 24 25 26 27 28 29
;; => 6 17 18 19 20 21 22 
;; =>  10 11 12 13 14 15 1
;; => (0 1 2 3 4 5 6 7 8 9

Is this a bug in the current code or a valid assumption that the handlers are making? It looks like cider-interactive-eval-handler is currently only called in conjunction with pr-request-map, all other commands which use pretty printing override the handler.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's what currently happens when the interactive-eval-handler is called with streaming enabled:

(defun cider-eval-defun-with-streaming ()
  "A hypothetical eval command"
  (interactive)
  (cider-interactive-eval
   (cider-defun-at-point)
   nil                                         ; <- the default interactive-eval-handler
   (cider-defun-at-point 'bounds)
   '(("nrepl.middleware.print/stream?" "1")    ; <- but with streaming
     ("nrepl.middleware.print/buffer-size" 20) ; <- and a small chunk size
     ("nrepl.middleware.print/print" "cider.nrepl.pprint/pr"))))

stream-overlay mov

Is this the intended behaviour? Obviously without the artificial delay, the user would only see the very last chunk in their overlay / minibuffer.

Accumulating the result and redisplaying the overlay on each chunk seems like a better solution:

accum-overlay mov

But in the first place, I believe none of the code paths in the current set of commands will lead to such a situation, unless some custom command like the above is defined.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you make a good point. Streaming of values wasn't enabled intentionally for interactive evaluation as I couldn't figure out a good way to print the result incrementally. I like your idea.

In an ideal world probably we should able to enable/disable streaming everywhere via defcustoms. For huge results definitely streaming results in a much nicer experience as you start seeing results faster.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the intended behaviour? Obviously without the artificial delay, the user would only see the very last chunk in their overlay / minibuffer.

And yeah - that's definitely not the intended behavior. :-) As mentioned about the streaming was disabled until we figure out how to best do it for buffer evaluations. Back then I recall I was planning to research potential options and as it usually happens - I never did. :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to insert this eval-into-register behaviour into every handler, or just the interactive-eval-handler?

Here's the full list:

  • cider-insert-eval-handler
  • cider-interactive-eval-handler
  • cider-load-file-handler
  • cider-eval-print-handler
  • cider-eval-print-with-comment-handler
  • cider-eval-pprint-with-multiline-comment-handler
  • cider-popup-eval-handler

Of these, I think it could be left out of the print-handler and popup-eval-handler, since these would be used for potentially large outputs. Accumulating a huge result in memory in addition to printing it out in chunks into an Emacs buffer could be a performance concern.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I added a commit which refactors the handlers to handle streamed values. Not too happy about the code duplication but there doesn't seem to be an clean way to abstract out all the (when cider-eval-register ... expressions

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems I forgot the respond here. The duplication is definitely not great, but I guess we can live with it, as the handler code rarely changes. I think that the ieval-handler and the load-file handler are definitely the most important ones when you might want to save the result to a register, but I guess for consistency it makes sense to extend this to more handlers as suggested by you. And yeah - we should avoid storing huge results in memory, knowing how poorly GC in Emacs performs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I removed it from the print-handler and popup-eval-handler, on the basis that the results are directly output to a buffer anyway, where they can be interacted with.

(setq res (concat res value))))
(lambda (_buffer out)
(cider-repl-emit-interactive-stdout out))
(lambda (_buffer err)
(cider-handle-compilation-errors err eval-buffer))
'())))
(lambda (_buffer)
(when cider-eval-register
(set-register cider-eval-register res))))))

(defun cider--emit-interactive-eval-output (output repl-emit-function)
"Emit output resulting from interactive code evaluation.
Expand Down Expand Up @@ -715,15 +729,12 @@ when `cider-auto-inspect-after-eval' is non-nil."
(end (or (car-safe (cdr-safe place)) place))
(beg (when beg (copy-marker beg)))
(end (when end (copy-marker end)))
(fringed nil))
(fringed nil)
(res ""))
(nrepl-make-response-handler (or buffer eval-buffer)
(lambda (_buffer value)
(if beg
(unless fringed
(cider--make-fringe-overlays-for-region beg end)
(setq fringed t))
(cider--make-fringe-overlay end))
(cider--display-interactive-eval-result value end))
(setq res (concat res value))
(cider--display-interactive-eval-result res end))
(lambda (_buffer out)
(cider-emit-interactive-eval-output out))
(lambda (_buffer err)
Expand All @@ -733,20 +744,31 @@ when `cider-auto-inspect-after-eval' is non-nil."
(cider--display-interactive-eval-result
err end 'cider-error-overlay-face)))
(cider-handle-compilation-errors err eval-buffer))
(when (and cider-auto-inspect-after-eval
(boundp 'cider-inspector-buffer)
(windowp (get-buffer-window cider-inspector-buffer 'visible)))
(lambda (buffer)
(lambda (buffer)
(if beg
(unless fringed
(cider--make-fringe-overlays-for-region beg end)
(setq fringed t))
(cider--make-fringe-overlay end))
(when (and cider-auto-inspect-after-eval
(boundp 'cider-inspector-buffer)
(windowp (get-buffer-window cider-inspector-buffer 'visible)))
(cider-inspect-last-result)
(select-window (get-buffer-window buffer)))))))
(select-window (get-buffer-window buffer)))
(when cider-eval-register
(set-register cider-eval-register res))))))


(defun cider-load-file-handler (&optional buffer done-handler)
"Make a load file handler for BUFFER.
Optional argument DONE-HANDLER lambda will be run once load is complete."
(let ((eval-buffer (current-buffer)))
(let ((eval-buffer (current-buffer))
(res ""))
(nrepl-make-response-handler (or buffer eval-buffer)
(lambda (buffer value)
(cider--display-interactive-eval-result value)
(when cider-eval-register
(setq res (concat res value)))
(when (buffer-live-p buffer)
(with-current-buffer buffer
(cider--make-fringe-overlays-for-region (point-min) (point-max))
Expand All @@ -756,12 +778,18 @@ Optional argument DONE-HANDLER lambda will be run once load is complete."
(lambda (_buffer err)
(cider-emit-interactive-eval-err-output err)
(cider-handle-compilation-errors err eval-buffer))
(or done-handler '())
(lambda (buffer)
(when cider-eval-register
(set-register cider-eval-register res))
(when done-handler
(funcall done-handler buffer)))
(lambda ()
(funcall nrepl-err-handler)))))

(defun cider-eval-print-handler (&optional buffer)
"Make a handler for evaluating and printing result in BUFFER."
;; NOTE: cider-eval-register behavior is not implemented here for performance reasons.
;; See https://github.com/clojure-emacs/cider/pull/3162
(nrepl-make-response-handler (or buffer (current-buffer))
(lambda (buffer value)
(with-current-buffer buffer
Expand All @@ -773,24 +801,28 @@ Optional argument DONE-HANDLER lambda will be run once load is complete."
(cider-emit-interactive-eval-output out))
(lambda (_buffer err)
(cider-emit-interactive-eval-err-output err))
'()))
()))

(defun cider-eval-print-with-comment-handler (buffer location comment-prefix)
"Make a handler for evaluating and printing commented results in BUFFER.
LOCATION is the location marker at which to insert. COMMENT-PREFIX is the
comment prefix to use."
(nrepl-make-response-handler buffer
(lambda (buffer value)
(with-current-buffer buffer
(save-excursion
(goto-char (marker-position location))
(insert (concat comment-prefix
value "\n")))))
(lambda (_buffer out)
(cider-emit-interactive-eval-output out))
(lambda (_buffer err)
(cider-emit-interactive-eval-err-output err))
'()))
(let ((res ""))
(nrepl-make-response-handler buffer
(lambda (_buffer value)
(setq res (concat res value)))
(lambda (_buffer out)
(cider-emit-interactive-eval-output out))
(lambda (_buffer err)
(cider-emit-interactive-eval-err-output err))
(lambda (buffer)
(with-current-buffer buffer
(save-excursion
(goto-char (marker-position location))
(insert (concat comment-prefix
res "\n"))))
(when cider-eval-register
(set-register cider-eval-register value))))))

(defun cider-maybe-insert-multiline-comment (result comment-prefix continued-prefix comment-postfix)
"Insert eval RESULT at current location if RESULT is not empty.
Expand Down Expand Up @@ -825,7 +857,9 @@ COMMENT-POSTFIX is the text to output after the last line."
(with-current-buffer buffer
(save-excursion
(goto-char (marker-position location))
(cider-maybe-insert-multiline-comment res comment-prefix continued-prefix comment-postfix))))
(cider-maybe-insert-multiline-comment res comment-prefix continued-prefix comment-postfix)))
(when cider-eval-register
(set-register cider-eval-register res)))
nil
nil
(lambda (_buffer warning)
Expand All @@ -834,6 +868,8 @@ COMMENT-POSTFIX is the text to output after the last line."
(defun cider-popup-eval-handler (&optional buffer)
"Make a handler for printing evaluation results in popup BUFFER.
This is used by pretty-printing commands."
;; NOTE: cider-eval-register behavior is not implemented here for performance reasons.
;; See https://github.com/clojure-emacs/cider/pull/3162
(nrepl-make-response-handler
(or buffer (current-buffer))
(lambda (buffer value)
Expand Down Expand Up @@ -1293,6 +1329,12 @@ passing arguments."
(form (format "(%s)" fn-name)))
(cider-read-and-eval (cons form (length form)))))

(defun cider-kill-last-result ()
"Save the last evaluated result into the kill ring."
(interactive)
(kill-new
(nrepl-dict-get (cider-nrepl-sync-request:eval "*1") "value")))

;; Eval keymaps
(defvar cider-eval-pprint-commands-map
(let ((map (define-prefix-command 'cider-eval-pprint-commands-map)))
Expand Down Expand Up @@ -1326,6 +1368,7 @@ passing arguments."
(define-key map (kbd "z") #'cider-eval-defun-up-to-point)
(define-key map (kbd "c") #'cider-eval-last-sexp-in-context)
(define-key map (kbd "b") #'cider-eval-sexp-at-point-in-context)
(define-key map (kbd "k") #'cider-kill-last-result)
(define-key map (kbd "f") 'cider-eval-pprint-commands-map)

;; duplicates with C- for convenience
Expand All @@ -1341,6 +1384,7 @@ passing arguments."
(define-key map (kbd "C-z") #'cider-eval-defun-up-to-point)
(define-key map (kbd "C-c") #'cider-eval-last-sexp-in-context)
(define-key map (kbd "C-b") #'cider-eval-sexp-at-point-in-context)
(define-key map (kbd "C-k") #'cider-kill-last-result)
(define-key map (kbd "C-f") 'cider-eval-pprint-commands-map)
map))

Expand Down
32 changes: 32 additions & 0 deletions doc/modules/ROOT/pages/usage/code_evaluation.adoc
View file
Open in desktop
Expand Up @@ -275,6 +275,34 @@ Additionally, there's the variable `cider-redirect-server-output-to-repl` that c

NOTE: The redirection functionality is implemented in `cider-nrepl` as nREPL middleware. If you're using CIDER without `cider-nrepl` no output redirection is going to take place.



=== Storing eval results

By default CIDER stores the return value of the most recent evaluation command
in the text register `e`. You can access these contents via `insert-register`
(kbd:[C-x r i]).

This is often useful for closer inspection or textual manipulation of a
transiently displayed eval result, without having to re-evaluate the form with a
specialized command like `cider-insert-last-sexp-in-repl`.

You can customize which register is used with the variable `cider-eval-register`, or set
it to `nil` to disable the feature.

[source,lisp]
----
(setq cider-eval-register nil)
----

TIP: The built-in xref:debugging/inspector.adoc[inspector] can be used to view
and navigate through complex nested results.

You can also use the command `cider-kill-last-result`(kbd:[C-c C-v k]) after any
eval command to store its result in the kill ring. This works even when the
`cider-eval-register` feature is disabled.


== Keybindings

You might have noticed that CIDER typically has 2-3 different keybindings for
Expand Down Expand Up @@ -383,6 +411,10 @@ kbd:[C-u C-c C-c]
| `cider-load-all-files`
| kbd:[C-c C-M-l]
| Load (eval) all Clojure files below a directory.

| `cider-kill-last-result`
| kbd:[C-c C-v k]
| Save the last evaluated result into the kill ring.
|===

TIP: You'll find all evaluation commands and their keybindings in the `CIDER Eval` menu.