Olical / conjure

Interactive evaluation for Neovim (Clojure, Fennel, Janet, Racket, Hy, MIT Scheme, Guile, Python and more!)
https://conjure.oli.me.uk
The Unlicense
1.74k stars 109 forks source link

stdio repl handling of both stderr and stdout for a form can be a problem sometimes #546

Open sogaiu opened 9 months ago

sogaiu commented 9 months ago

If using Janet's stdio repl [1], evaluating the following form leads to on-stray-output being called sometimes:

(do                                                     
  (print "out")
  (error "err"))

IIUC, this occurs because a callback (cb) gets cleared here as a result of on-stdout triggering before on-stderr (and they both call on-message).

By the time on-stderr gets called, cb ends up being bound to on-stray-output because of this.

In my limited testing at the command line (evaluating the same form), stdout output appears to complete after stderr output which is the reverse order of the callbacks being called. I suppose it's possible that that's just the way it could end up being when using event-based things like libuv?


[1] Possibly this may be an issue for other languages too as relevant code lives in fnl/conjure/remote/stdio.fnl.

russtoku commented 5 months ago

If you're up to modifying the Janet client code, you could try replacing:

https://github.com/Olical/conjure/blob/4880144f13a7693cded529b302de491bfbaedbd6/fnl/conjure/client/janet/stdio.fnl#L5

with:

https://github.com/Olical/conjure/blob/4880144f13a7693cded529b302de491bfbaedbd6/fnl/conjure/client/sql/stdio.fnl#L5

Please see: https://github.com/Olical/conjure/blob/4880144f13a7693cded529b302de491bfbaedbd6/fnl/conjure/client/sql/stdio.fnl#L96

for how things are handled in the SQL client. This might work with a Janet REPL.

This is suggested as a simple work-around for handling other stdio clients. When @Olical completes his modularisation work (https://github.com/Olical/conjure/discussions/500), hopefully, we can improve how the various REPLs work with Conjure.

sogaiu commented 2 months ago

@russtoku Sorry, for the late response and thanks for these tips!

I had started thinking about doing something like unrepl, i.e. an upgradeable repl, and coincidentally, someone pointed out @andreyorst's fennel-proto-repl-protocol idea. May be you're familiar with it already [1]?

I don't know what the status of that is, but it (or some of the contained ideas) seemed worth considering.

Didn't know about #500 -- will see what I can make of it (^^;


[1] Here are a couple of related blog posts for future readers :)

russtoku commented 2 months ago

@sogaiu Thanks for sharing! I wasn't aware of fennel-proto-repl. Excellent posts!

I have attempted to diagram some of the REPL interaction in Conjure. Unfortunately, I'm still very much in the dark about things.

andreyorst commented 2 months ago

Jul 6, 2024 14:01:05 sogaiu @.***>:

@russtoku[https://github.com/russtoku] Sorry, for the late response and thanks for these tips!

I had started thinking about doing something like unrepl[https://github.com/Unrepl/unrepl], i.e. an upgradeable repl, and coincidentally, someone pointed out @andreyorst[https://github.com/andreyorst]'s fennel-proto-repl-protocol[https://github.com/andreyorst/fennel-proto-repl-protocol] idea. May be you're familiar with it already [1]?

I don't know what the status of that is, but it (or some of the contained ideas) seemed worth considering.


[1] Here are a couple of related blog posts for future readers :)

— Reply to this email directly, view it on GitHub[https://github.com/Olical/conjure/issues/546#issuecomment-2211738223], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AEUROTYT2WCQJ6QSOWRVT5TZK7E7BAVCNFSM6AAAAABAYHX72KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMJRG4ZTQMRSGM]. You are receiving this because you were mentioned.

I believe nothing stops conjure from using fennel-proto-repl protocol implementation. There's already one neovim client that does, and the author contributed some fixes that made it more robust in neovim's environment.

If you need help understanding the protocol, and how to bend it to suit conjure's needs, feel free to ask! I've suggested this some time ago too: https://github.com/Olical/conjure/discussions/478

sogaiu commented 2 months ago

@russtoku I'm probably much more in the dark than you about conjure's workings. Are any of the diagrams you hinted at available for viewing somewhere?

sogaiu commented 2 months ago

@andreyorst Thanks for chiming in -- perhaps I should have searched the discussions first (^^;

Regarding "already one neovim client", may be you meant this repository?

andreyorst commented 2 months ago

Regarding "already one neovim client", may be you meant this repository[https://github.com/HiPhish/fennel-repl.nvim/]?

Yes, that's it

russtoku commented 1 month ago

@russtoku I'm probably much more in the dark than you about conjure's workings. Are any of the diagrams you hinted at available for viewing somewhere?

Sorry, I replied in Discord instead of here so just pasting in my replies from Discord.

In my frustration, I sent them to the trash.

I did keep some notes on the ways that Neovim plugins send stuff to a sub-process. https://russtoku.github.io/posts/nvim-and-repls.html

andreyorst commented 1 month ago

I did keep some notes on the ways that Neovim plugins send stuff to a sub-process. https://russtoku.github.io/posts/nvim-and-repls.html

To be clear , fennel-proto-repl doesn't have to be running in a subprocess, if the plain stdio fennel repl doesn't. The proto-repl implemented in a way where you can "upgrade" an ordinary fennel repl to use a protocol. In other words, Conjure just starts an oedinary fennel repl, like it does already, and sends the protocol code into it. After the protocol code was evaluated, the repl starts working in terms of messages, so all conjure needs is to send messages in the protocol format, and parse the oncoming messages.

So, I believe Conjure doesn't have to support anything it already doesn't to work with proto-repl. Most of the code for nREPL message handling can be adapted for stdio.