Add queue for tap values in clojure client

[EgorKulbachka]

I absolutely loved the idea of #220 and decided to give it a shot.

Not sure if I got the idea right, especially I struggled with choosing hotkeys, but I intend to play around with it and see how it works for myself.

Would love any kind of feedback on that.

[Olical]

Woah this is really cool! I think it's a great idea and the implementation looks good. I feel a little iffy any time I introduce code that's sent on connection, just in case it breaks something for the user, but as long as it can be turned off and we test it out it should be fine. Just something that adds some level of risk.

I think the queue with a way to peek + print it all is great, the only thing I'm thinking is that we might need more unique namespaces (conjure might be too short and could collide with something) and maybe the viewing of a queue should also drain it. So yes there's a mapping to drop the current contents, but I feel like view should also clear.

So it's kind of a pop/pop-all rather than peek/print-all. More side effects here will make more sense to the user I think.

I think starting with a manual way for users to pop values out into their logs is good. Then it's a very easy next step to add some sort of automated poll / check after evals so that tap values are automatically printed on the user's behalf as they come in.

Will have a review and a test run with it ASAP, but on the whole this looks fantastic, thank you so much.

[Olical]

Merged and modified, thank you very much! This is super cool. I simplified things a little so there's a little less surface area / room for confusion (if it's off by default I don't think we actually save much but I'll get people opening issues asking why the mapping doesn't work 😅 ), I quite like where it's at right now. Not your exact original vision, but close to it, I hope you don't mind the changes I made.

I merged the initial Clojure code we send into the conjure.internal namespace that was already being sent to the REPL. I also updated that code to fix some issues with babashka, so now the whole "swap to another ns and back" actually works in all REPL types. What we had before threw errors and broke bb REPLs! (you can't resolve *ns* in bb and if you in-ns to the value of *ns* it breaks the REPL. You have to get the SYMBOL and work with that.

There's only one mapping, it prints and clears the queue because I think that's what 90% of the use cases will be. You can access the last value you just printed with *1 if required and you can access @conjure.internal/tap-queue! in the REPL if you need programmatic access.

Please check out the changes on develop / 4237619 and let me know what you think / if it's working okay for you.

Thanks again for adding this feature, it's really cool!

[EgorKulbachka]

Thanks for merging this so quickly, I'll try to use updated version now, changes you've made make a lot of sense.
Especially replacing everything with single "print and clear" is quite elegant solution which I overlooked, it makes things much easier for sure.

[EgorKulbachka]

@Olical I actually just played around with it and I think there is a bug where each time I connect to nrepl it adds one more tap processing function, so single tap will add multiple values onto the queue.
I believe this function has to be defonce otherwise remove-tap doesn't remove old tap handler since we re-define function and reference to the old one will be lost.

[Olical]

OH! That's why that was a defonce! I should've thought about that instead of changing it, silly me. I'll put that back soon / accept a PR if you want to do it and use it right now. Otherwise I can probably do it tomorrow morning. Great catch!

[Olical]

Fixed on develop, thanks for catching it. I knew something felt odd about the use of defonce when I first looked at it, like it had to be deliberate. I should've asked you why rather than just change it.

I also added a comment in the code explaining why it's like this now.