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

add an interactive function cider-eval-last-sexp-to-kill-ring #3143

Conversation

blak3mill3r
Copy link

This is my first attempt to contribute to cider, please consider this PR a draft. I am no elisp expert, I welcome feedback. I read the CONTRIBUTING guidelines but I must confess that I did not understand all of them.

This is a feature that I'd been wanting: eval, storing the eval result in the kill ring

takes an optional pretty argument

stores the eval result in the kill ring

there was some discussion about this feature here
#2580

It's based on master as of this morning, I'm using emacs built from source fairly recently:

emacs-27.1-11299-g5b250ca79b

I'm using native-compilation

I did not add a default keybinding in this commit. The keybindings I use for cider are very different from the defaults and so I don't feel qualified to suggest a reasonable default keybinding for this.

takes an optional `pretty` argument

stores the eval result in the kill ring

there was some discussion about this feature here
clojure-emacs#2580
(if (derived-mode-p 'cider-clojure-interaction-mode)
(format "\n%s\n" value)
value))
(copy-region-as-kill (point-min) (point-max))))
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't look correct to me as you'll be adding to the clipboard each chunk of the response (provided it's big enough to the split into several response messages). Normally you'd want to collect all the chunks until you get "done" and only then copy something to the clipboard.

Copy link
Member

Choose a reason for hiding this comment

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

Also - it seems to me it would still be a goo idea to echo somewhere the result, so you'd know what ended up in the kill-ring without having to check it.

(cider-interactive-eval nil
(cider-eval-kill-ring-handler)
(cider-last-sexp 'bounds)
;; this says that it does NOT require pretty printing... add an option?
Copy link
Member

Choose a reason for hiding this comment

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

I'm not quite sure what this is supposed to mean.

@yuhan0
Copy link
Contributor

yuhan0 commented Feb 7, 2022

I think it would be better to reserve one of Emacs' built-in registers for this, and always copy the result of any interactive eval into it - something like

(defcustom cider-eval-register ?e
  "When non-nil, save the last eval result into this register"
  :type '(choice character (const nil)))

Then instead of having to consciously call a dedicated eval-to-kill-ring command/ family of commands, one could decide after any regular cider-eval-defun or eval-buffer or eval-last-sexp-up-to-point etc. that the result was worth saving, and simply yank it from the e register. (C-x r i e / " e p with evil)

@yuhan0
Copy link
Contributor

yuhan0 commented Feb 7, 2022

Or alternatively, have a command which prints the repl's *1 value into the kill ring, similar to cider-inspect-last-result.

@bbatsov
Copy link
Member

bbatsov commented Feb 7, 2022

Yeah, I think that's a better and simpler approach. PR welcome!

@yuhan0
Copy link
Contributor

yuhan0 commented Feb 7, 2022

Which approach would you prefer? Auto saving to register (might be less discoverable) or adding a cider-kill-last-result command (more keystrokes to execute)

@bbatsov
Copy link
Member

bbatsov commented Feb 9, 2022

@yuhan0 I was actually thinking of doing both, as they are not mutually exclusive. :D Probably with a defcustom to enable the auto-save to register behavior.

@bbatsov
Copy link
Member

bbatsov commented Feb 21, 2022

@yuhan0 Any chance you'll find a time for this soon? I'm thinking of cutting a new CIDER release in March, and it'd be nice if this made the cut.

@yuhan0
Copy link
Contributor

yuhan0 commented Feb 21, 2022

Oh yes, I'll submit a PR in the next day or two.

@bbatsov
Copy link
Member

bbatsov commented Mar 9, 2022

Closing this in favour of #3162.

@bbatsov bbatsov closed this Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants