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

Feature request: support VSCode Insiders edition #24

Closed
postylem opened this issue Aug 29, 2022 · 8 comments
Closed

Feature request: support VSCode Insiders edition #24

postylem opened this issue Aug 29, 2022 · 8 comments

Comments

@postylem
Copy link

I'd like to open a ssh connection to a compute node in VSCode Insiders, rather than VSCode (that is, I think as simple as using cli command code-insiders rather than code). From what I can tell, there isn't currently a way to do this, as running

mila code $PATH_ON_CLUSTER

will salloc a node $NODE_NAME, and try to connect with it using the command code -nw --remote ssh-remote+$NODE_NAME $PATH_ON_CLUSTER. I'd like to have the option to use code-insiders ... instead.

To enable this is I think as simple as modifying "code", -> "code-insiders", in milatools/cli/__main__.py (line 277)

here.run(
"code",
"-nw",
"--remote",
f"ssh-remote+{node_name}",

But ideally, I'd be able to do this with a different command, like

mila code-insiders $PATH_ON_CLUSTER

Is there a reason why this isn't a good idea?

@postylem postylem changed the title support VSCode Insiders edition Feature request: support VSCode Insiders edition Aug 29, 2022
@breuleux
Copy link
Member

I don't think it should be a separate command, but it could be an option e.g. --command code-insiders and/or an environment variable e.g. $MILATOOLS_CODE_COMMAND.

@breuleux
Copy link
Member

Although, I feel like you would typically not use both code and code-insiders, so couldn't you alias the former to the latter?

@postylem
Copy link
Author

The first thing I tried was to alias code=‘code-insiders’ but for a reason I don’t understand it doesn’t work; it still runs the command as code. I do think though that if one were potentially testing some feature or other in code-insiders and get really otherwise using standard code, it would be useful to have the switch available as a cli option.

@breuleux
Copy link
Member

Ah yeah I think it's just because aliases are processed by the shell and code is executed with shell=False.

@postylem
Copy link
Author

Ah, okay. And I imagine subprocess.run( ... , shell=True) is probably not the right solution here. Would an option to specify the command option or reference an environment variable be the best?

@breuleux
Copy link
Member

breuleux commented Sep 1, 2022

shell=True can work, but it requires some care in escaping special characters. I think an option and environment variable are a better solution.

@breuleux
Copy link
Member

breuleux commented Sep 1, 2022

I published version 0.0.11, which adds the flag and environment variable along with another fix.

@postylem
Copy link
Author

Just to be clear for anyone visiting this thread, as of version 0.0.11, to use VSCode Insiders with milatools, the following will work:

Set environment variable

export MILATOOLS_CODE_COMMAND=code-insiders

Then run

mila code .

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

No branches or pull requests

2 participants