DavesCodeMusings / mpremote-vscode

Visual Studio Code extension for mpremote Python module
BSD 2-Clause "Simplified" License
10 stars 1 forks source link

Restart REPL before uploading #25

Open daftscience opened 2 months ago

daftscience commented 2 months ago

Hi, I just discovered this extension and it's pretty great. I'm hoping it will be a huge improvement over Pymakr.

I might be missing something or haven't figured out the best workflow yet. However, I'm trying to use this in conjunction with vscode-file-watcher. I have that extension set up to run mpremote.upload when I save a file. It seems to work pretty well; the only downside is that it doesn't work when I have the REPL open.

Is it possible to have that command stop the REPL before uploading?

DavesCodeMusings commented 2 months ago

This is something I've looked at in the past. I agree, it would be nice to close the REPL prompt before other commands are sent. The trouble I ran into was how to detect if the REPL is running. Otherwise, the logic would be pretty simple:

if (repl_is_running == true) {
  close_repl
}
execute mpremote command

The missing piece is repl_is_running.

It can't be a flag that's set when the REPL is started, because someone could CTRL-X out of the REPL and then the flag would be out of sync with the actual state. So it comes down to determining what process is running in the VS Code terminal window, and I have not figured out how to do that.

If anyone has an idea on how to reliably determine if the REPL is running in the VS Code terminal, please let me know.

daftscience commented 1 month ago

Thanks. I haven't done any extension development before so I'm a bit out of my depth here. I did poke around the code a bit and it looks like the terminal is specific to the extension. Would adding something like this work?

uploadRepl(port: string, localPath: string, remotePath: string) {
      if (port && localPath && remotePath) {
          // send ctrl+x to to the terminal 
          this.terminal.sendText(`${String.fromCharCode(29)}`); // send command to exit repl
          this.terminal.sendText(`${this.mpremote} connect ${port} cp '${localPath}' ':${remotePath}'`);
          this.repl(port);
          this.terminal.sendText(`\u0004`); // send command to soft reboot. 
      }
  }

From my testing it appears to work regardless of if REPL has been terminated manually or not. It's not very elegant, but it appears to work. I can see sending too many requests at a time could cause some issues though.

daftscience commented 1 month ago

I've found the flaws in that approach. However, I've forked your repo and have created a few async functions to handle this. It's still not terribly clean, but it does successfully disconnect and reconnect.

`
    uploadRepl(port: string, localPath: string, remotePath: string) {
        if (port && localPath && remotePath) {
            this.closeRunningTerminal().then(() => {
                this.createTerminal().then(() => {
                    this.terminal.sendText(`${this.mpremote} connect ${port} cp '${localPath}' ':${remotePath}'`);
                    this.repl(port);
                    this.terminal.sendText('\x04', false);
                });
            });
        }
    }
    async closeRunningTerminal() {
        vscode.window.terminals.forEach(terminal => {
            if (terminal.name === 'mpremote') {
                terminal.dispose();
            }
        });
    }
    async createTerminal() {
        // create a new terminal window
        this.terminal = vscode.window.createTerminal('mpremote');;
        this.terminal.show();
    }

`

DavesCodeMusings commented 1 month ago

I've been thinking this over. Your first solution seems to be simplest:

this.terminal.sendText(`${String.fromCharCode(29)}`); // send command to exit repl

This could be sent before any other (non-repl) mpremote command. It would exit the REPL if it's running and probably only result in an extra blank line in the terminal if it's not running.

I could create an extension checkbox option something like Force REPL exit before running command so if it got too annoying, it could be switched off.

So a command like upload would look like this:

upload(port: string, localPath: string, remotePath: string) {
    if (port && localPath && remotePath) {
        if (forceExitREPL) {
            this.terminal.sendText(`${String.fromCharCode(29)}`); // send command to exit repl
        }
        this.terminal.sendText(`${this.mpremote} connect ${port} cp '${localPath}' ':${remotePath}'`);
    }
}

Any thoughts on this proposed solution?

daftscience commented 1 month ago

Yeah, I like it. I was running into issues with sending ctrl commands to the terminal which is why I decided to just nuke the terminal instead. I agree, sending the ctrl chars to quit REPL is a more graceful way of handling it.

DavesCodeMusings commented 1 month ago

When you say you had problems sending control commands, did you mean:

this.terminal.sendText(`${String.fromCharCode(29)}`); // send command to exit repl

Hopefully that's not it. I was planning to send the CTRL+X that way.

daftscience commented 1 month ago

yeah, I can't remember all the details but it seemed inconsistent. I don't know if it was just poorly implemented on my part though.