Skip to content

Add command to print the current value of the inspector. #3810

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

r0man
Copy link
Contributor

@r0man r0man commented Apr 13, 2025

This PR adds the interactive cider-inspector-print-current-value command that prints the current value of the inspector to the cider-inspector-value-buffer with the usual pretty printing machinery.

The NREPL part is here: clojure-emacs/cider-nrepl#933

When pressing "P" in the inspector the currently inspected value is printed in a new popup buffer (on the left side in the picture).

cider-inspect-print-current-value

I see it as the inspectors brother of cider-pprint-eval-last-sexp which I use all the time.

This is the functionality I initially suggested, before working on the pretty mode. I think it is still useful. What I would like to have is a quick way to pretty print the current value of the inspector in the same familiar way my eye is used to, as the Clojure pretty printer (or what we are using in CIDER).

The new pretty mode does print more pretty, but differently, it uses "inspector mode" printing.

That's why I think this could be a useful addition.

Wdyt?


Before submitting the PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):

  • The commits are consistent with our contribution guidelines
  • You've added tests (if possible) to cover your change(s)
  • All tests are passing (eldev test)
  • All code passes the linter (eldev lint) which is based on elisp-lint and includes
  • You've updated the changelog (if adding/changing user-visible functionality)
  • You've updated the user manual (if adding/changing user-visible functionality)

@bbatsov
Copy link
Member

bbatsov commented Apr 22, 2025

I'm fine with this suggestion. @alexander-yakushev what do you think?

@alexander-yakushev
Copy link
Member

With pretty-printing mode now merged, do you still desire to have this feature, @r0man? If so, we'll need some other key binding.

@r0man
Copy link
Contributor Author

r0man commented Apr 22, 2025

Hi @alexander-yakushev and @bbatsov, yes, I'm still interested in it. I take a look at it later or tomorrow and look for a new keybinding.

@r0man r0man force-pushed the inspect-pprint-current-value branch 2 times, most recently from 6e82e37 to ea70033 Compare April 23, 2025 06:50
@@ -146,6 +147,7 @@ Can be turned to nil once the user sees and acknowledges the feature."
(define-key map [(shift tab)] #'cider-inspector-previous-inspectable-object)
(define-key map "p" #'cider-inspector-previous-inspectable-object)
(define-key map "P" #'cider-inspector-toggle-pretty-print)
(define-key map "x" #'cider-inspector-print-current-value)
Copy link
Member

Choose a reason for hiding this comment

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

😬
I was actually eyeing x for another pretty-printing functionality we might do in the future – eXpanding arbitrary values at the cursor point, pretty-printing just that value in an otherwise regular inspector view.

What do you think about C-p? It will still be Printing-related this way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, C-p is usually bound to previous-line. So maybe better not?

Copy link
Member

Choose a reason for hiding this comment

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

I'd go with something like r (I guess render can be a mnemonic for it) - in the absence of meaningful names I go for those that are easy to type.

Copy link
Member

Choose a reason for hiding this comment

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

M-p?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about C-c C-p? We use that keybinding in cider-mode:

cider-mode.el:502:    (define-key map (kbd "C-c C-p") #'cider-pprint-eval-last-sexp)

It's not used in inspector mode, and its similar to that command?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that's a great suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went with C-c C-p now. Are you fine with this @bbatsov ?

Copy link
Member

Choose a reason for hiding this comment

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

That's normally a minor mode keybinding (C-c something), so it's a bit weird IMO, but I don't have strong reservations for it.

Copy link
Member

@alexander-yakushev alexander-yakushev left a comment

Choose a reason for hiding this comment

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

Looks very clean now!

This PR adds the interactive `cider-inspector-print-current-value`
command that prints the current value of the inspector to the
`cider-inspector-value-buffer` with the usual pretty printing
machinery.
@r0man r0man force-pushed the inspect-pprint-current-value branch from d312f55 to 0b90313 Compare April 23, 2025 08:54
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