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 optional tap> output to the inspector #3055

Open
Tracked by #3090
bsless opened this issue Sep 18, 2021 · 10 comments
Open
Tracked by #3090

Add optional tap> output to the inspector #3055

bsless opened this issue Sep 18, 2021 · 10 comments

Comments

@bsless
Copy link

bsless commented Sep 18, 2021

Hello,
Seems like tap> and the inspector can work very well together, and adding such capability takes only a few lines of code.
I propose a new interactive function cider-inspect-tap which will display an atom updated by a tap added by cider.

In the Clojure side, this is all the needed implementation:

(defn queue
  [& args]
  (into (clojure.lang.PersistentQueue/EMPTY) args))

(defn bounded-conj
  [^long n coll x]
  (let [b (== (count coll) n)]
    (cond-> (conj coll x)
      b pop)))

(def queue-size (atom 16))
(def taps-queue (atom (queue)))

(defn set-size! [n] (reset! queue-size n))

(defn reset-queue! [] (reset! taps-queue (queue)))

(defn qtap! [x] (swap! taps-queue #(bounded-conj @queue-size % x)))

(defn register! [] (add-tap qtap!))

(defn deregister! [] (remove-tap qtap!))

(defn view! [] (reverse (deref taps-queue)))

In CIDER, a minimal implementation would be

;;;###autoload
(defun cider-inspect-tap ()
  "View taps queue."
  (interactive)
  (cider-inspect-expr "(view!)" (cider-current-ns)))

I would happily PR this, but I wanted to start by opening an issue to track discussion instead of having it on Slack, and to ask a few questions regarding design.

  • Should Clojure code be added to orchad, cider-nrepl, or just injected via CIDER?
  • Should the code be in the same name space and package as the inspector or on its own?
  • How should older versions of Clojure with no tap> support be handled?
  • Should the inspected expression be defined in elisp as a var? const? Should I use requiring-resolve?

These are the most important questions as far as I can see for now.

Let me know what you think and I'll happily start on an implementation.

Cheers

@bbatsov
Copy link
Member

bbatsov commented Oct 2, 2021

Should Clojure code be added to orchad, cider-nrepl, or just injected via CIDER?

The Clojure part of the inspector lives in Orchard, so it should be updated there.

Should the code be in the same name space and package as the inspector or on its own?

Same ns is fine.

How should older versions of Clojure with no tap> support be handled?

Probably this should be some no op there. I think we support only Clojure 1.8+. When was it added?

Should the inspected expression be defined in elisp as a var? const? Should I use requiring-resolve?

I don't quite get the question. More details please.

Sorry for the slow response. I've been traveling a bit lately and I've spent little time in front of a computer. @alexander-yakushev might have something to say about this as well, given he's been working the most on improving the inspector.

@bsless
Copy link
Author

bsless commented Oct 3, 2021

I don't mind the slow response, maintainers have a life, too
To clarify what I meant with my last question,
when I call cider-inspect-expr, should that expression be a hard coded string? defined with defvar or defconst? Should it be a fully qualified name, i.e. some.ns/view! or just view!? Should the code I send to inspect be wrapped with requiring-resolve or can I assume it will be loaded?

@bbatsov
Copy link
Member

bbatsov commented Oct 27, 2021

To clarify what I meant with my last question,
when I call cider-inspect-expr, should that expression be a hard coded string?

I'd go with a defvar.

Should it be a fully qualified name, i.e. some.ns/view! or just view!?

Fully-qualified names work best.

Should the code I send to inspect be wrapped with requiring-resolve or can I assume it will be loaded?

Well, that depends. I just realized that you're thinking of evaluating the code directly, but so far there's no place where CIDER just evals some code that lives in say cider-nrepl. Might be prudent to wrap this in some nREPL op, which would also solve all the other points you mentioned (it will just call that view! function you proposed).

@bbatsov bbatsov mentioned this issue Nov 5, 2021
5 tasks
@hugoduncan
Copy link
Member

hugoduncan commented Dec 7, 2021

Would it be possible to have cider watch the var, and automatically display changes in the inspector?

For that to work well, the inspector would probably need to keep, and allow navigation through, history. Presumably it could keep history using weak references.

@bsless
Copy link
Author

bsless commented Dec 7, 2021

To not override the default inspector's behavior, to always display the last evaluation result, I think a good way to go about it would be adding a *cider-tap-inspector*

@bbatsov
Copy link
Member

bbatsov commented Dec 31, 2021

Something like this would be fine by me.

@stale
Copy link

stale bot commented Apr 17, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

@stale stale bot added the stale label Apr 17, 2022
@darkleaf
Copy link
Contributor

up

@vemv
Copy link
Member

vemv commented Sep 26, 2023

@bsless your code works great as-is.

Would you like to create a PR?

If not, I can productionize it :)

If you have since added new tweaks, please edit your OP reflecting them.

Cheers - V


Edit: I crowdsourced extra design/impl considerations here https://clojurians.slack.com/archives/C03S1KBA2/p1695754708326079

@vemv vemv pinned this issue Sep 26, 2023
@raszi
Copy link

raszi commented Jan 22, 2024

I just integrated this solution over the weekend, and it works great! Thank you very much!

I can hardly wait to see this in CIDER! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants