CFiggers / vscode-janet-plus-plus

Janet language support for Visual Studio Code
Other
11 stars 3 forks source link

Evaluation sending code to the Terminal #28

Open noncom opened 1 month ago

noncom commented 1 month ago

Hello! Thanks so much for the extension!

There's one problem I'm finding, however. The extension seems to think that if any terminal is open, then it's necessarily a Janet REPL, while it's not always so. For example, if you open the REPL, and then exit Janet, or if you just have an unrelated terminal open, and you evaluate something, it just sends everything into the terminal, despite that it's just the shell, and not Janet.

I don't know if there's an easy, or even any, way of determining if a terminal is actually a Janet REPL, or if there's any other possible solution to this, but it would've been great if this could be somehow fixed.

CFiggers commented 1 month ago

Hi there! Glad you're enjoying Janet++.

What you're describing is a side-effect of the extremely unsophisticated means of REPL evaluation that Janet++ currently uses. We essentially just open Janet in an integrated terminal window and chuck code at it. There's no validation, no checking to make sure the terminal window has Janet running, no nothin'.

Someone better versed with the VS Code API might be able to do what you're saying much more easily than I would be able to (purely because of my own ignorance of how to actually do it)—I'm limited more by my own lack of skill than the true restrictions of the platform per se. If you know VS Code's API better than I do, I'm happy to review PR's. :)

(For the record, in the future I want to make code evaluation a lot smarter using Janet LSP. But we're a ways off from getting that working yet.)

In the meantime, I can hopefully help somewhat by pointing out that the extension does not send code to "ANY" open terminal session. The extension will always spawn a specific terminal that it uses for evaluation—you can tell which one that is by looking at the terminal pane's name:

image

If you have shrunk that little sidebar, you can still see the name if you hover over it:

image

Janet++ will ONLY send code for evaluation to THAT SPECIFIC terminal pane. So, to keep streams from getting crossed, it is my recommendation to only have a Janet session running in that terminal. There are of course cases where you need to kill the Janet process (to get a clean reset of REPL state, or if you start an infinite loop that must be terminated). But if that happens, you should immediately start Janet back up in that window. If you want to do other stuff with the integrated terminal in VS Code, I recommend spawning a new one for that purpose. I treat the "Janet REPL" terminal as reserved purely for REPL interaction and that avoids pretty much all issues like the ones you describe.

noncom commented 1 month ago

Ah, the fact that the Janet++ looks for the named terminal was missed by me! Then yeah, if I follow these simple rules, it's going to be much easier, haha. Thanks! The issue is not resolved per se, but the appropriate know-how reduces its impact to almost zero, so I'm not sure about the status of this issue.

Also, I'm myself absolutely ignorant about how VSCode extensions work, and from my attempts to debug a few extensions before, I'd say that it's not so simple.

I've searched a bit, and found some references that might be relevant, but I currently I have zero knowledge if these avenues are even valid or not. I'll just leave the links as a reference, maybe even for my future self, if I come back to this:

CFiggers commented 1 month ago

The issue is not resolved per se, but the appropriate know-how reduces its impact to almost zero, so I'm not sure about the status of this issue.

It seems to me there is some improvement possible here, so let's keep the issue open for a bit. :)

Thanks for looking into those options. That's definitely effort you were/are under no obligation to put in, so the fact that you're going out of your way to do research like that is encouraging and appreciated. :)

Here's how I'm thinking. I already know that long-term I want to thoroughly overhaul evaluation to run through Janet LSP—that's essentially an equivalent to your option 3 (only instead of an nREPL I'm going to run it through the LSP—and you're quite right, "sounds complicated" is correct. I know there's some complexity to manage around handling errors and keyboard interrupts. There's probably more complexity that I'm currently unaware of. I'll probably have to significantly expand my understanding to get that implemented). In the mean time, if there is a way to improve what we have iteratively without needing a complete re-design, I'd generally prefer to do that—if there's not really a simple option like that, then it would be better from my perspective to leave it as-is (I just don't want to put in a ton of effort that will be wasted once I get around to the "real" overhaul).

Looking through the VS Code docs, I see that it's pretty simple to get TerminalExitStatus from an active Terminal, once you have a reference to it (which I need to do anyway in order to send Janet code to it—docs here). It should be that a running Janet process will not have a TerminalExitStatus (the REPL process will not have exited). So maybe, we can just add in a check for TerminalExitStatus on the Janet REPL terminal. If it's undefined, assume that Janet is running and send the code as usual. If it has a TerminalExitStatus, then assume the Janet process has exited for some reason and send janet to that terminal before sending additional code to it.

It's not a perfect solution (if the user starts up a different long-running process then the extension will be confused and assume that process must be Janet). But, it would be a pragmatic improvement, would be simple to implement, and might help in >50% of cases.