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

[inspector] Use orchard.inspect/refresh instead of individual setters #877

Merged
merged 2 commits into from
May 30, 2024

Conversation

alexander-yakushev
Copy link
Member

@alexander-yakushev alexander-yakushev commented May 14, 2024

Sister to clojure-emacs/orchard#269. This is how it would look like on the cider-nrepl side. Old ops are retained for compatibility and marked as deprecated.

@alexander-yakushev alexander-yakushev marked this pull request as draft May 14, 2024 12:49
src/cider/nrepl.clj Outdated Show resolved Hide resolved
@@ -335,7 +335,7 @@ if applicable, and re-render the updated value."
:requires {"session" "The current session"}
:returns inspector-returns}
"inspect-refresh"
{:doc "Re-renders the currently inspected value."
{:doc "Updates inspector with the provided config and re-renders the current value. Supports the same config keys as {\"op\" \"eval\", \"inspect\" \"true\"}."
Copy link
Member

Choose a reason for hiding this comment

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

While we're here you could add :optional and refer to those config keys

(good to keep them DRY with a simple def)

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean defing them in cider.nrepl.middleware.inspect and referring to them here?

Copy link
Member Author

Choose a reason for hiding this comment

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

You probably mean using them twice, in inspect-refresh and in eval inspect true, but I can't find any mention of the latter in the docs.

Copy link
Member

Choose a reason for hiding this comment

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

mmm I was assuming that the config keys {\"op\" \"eval\", \"inspect\" \"true\"} were already documented somewhere here. Maybe it's not the case as eval is not defined in cider-nrepl, IDK

Feel free to take it from here as deemed best

@alexander-yakushev alexander-yakushev force-pushed the inspect-refresh branch 2 times, most recently from a2276e0 to 86c8562 Compare May 14, 2024 14:01
@alexander-yakushev alexander-yakushev force-pushed the inspect-refresh branch 2 times, most recently from 849620a to ba445ca Compare May 30, 2024 14:39
@alexander-yakushev alexander-yakushev marked this pull request as ready for review May 30, 2024 14:39
@alexander-yakushev
Copy link
Member Author

Ready to be reviewed now. I also added the support for new :view-mode option as now this is just another argument to eval or inspect-refresh.

@alexander-yakushev
Copy link
Member Author

Actually, sorry, let me walk back that decision. :view-mode will ship in a separate PR.

@alexander-yakushev alexander-yakushev merged commit 07cf028 into master May 30, 2024
55 checks passed
@alexander-yakushev alexander-yakushev deleted the inspect-refresh branch May 30, 2024 15:29
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.

2 participants