-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Enable the terminal command for all remote names #7859
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
base: main
Are you sure you want to change the base?
Enable the terminal command for all remote names #7859
Conversation
import iconv from "iconv-lite"; | ||
import childProcess from "node:child_process"; | ||
import os from "node:os"; | ||
import util from "node:util"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not used
I have read the CLA Document and I hereby sign the CLA agrabauskas seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. |
} from "../../util/processTerminalStates"; | ||
import { getBooleanArg, getStringArg } from "../parseArgs"; | ||
|
||
const asyncExec = util.promisify(childProcess.exec); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not used
const ideInfo = await extras.ide.getIdeInfo(); | ||
const toolCallId = extras.toolCallId || ""; | ||
|
||
if (ENABLED_FOR_REMOTES.includes(ideInfo.remoteName)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actual change is removing this if.
All lines below are just indentation changes
// For remote environments, just run the command | ||
// Note: waitForCompletion is not supported in remote environments yet | ||
await extras.ide.runCommand(command); | ||
return [ | ||
{ | ||
name: "Terminal", | ||
description: "Terminal command output", | ||
content: | ||
"Terminal output not available. This is only available in local development environments and not in SSH environments for example.", | ||
status: "Command failed", | ||
}, | ||
]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing fall-back
@etherandrius I think the original reason for this logic was to enable running commands in the remote environment, i.e. less as a fallback for environments that don't support it and more of a way to enable running commands in remotes at all (through IDE interface). It seems like this has slowly been watered down so not all commands are run locally, partly because the support for local commands is so much better now (partial output, backgrounding, etc.) @chezsmithy you've done a lot of work here, noticed your reaction, do you have any thoughts on running commands remote vs locally when in a remote filesystem? |
@RomneyDa there are explicit ways to use local vscode with remote terminals. I've had a few ideas I've tinkered with but it gets odd when vscode makes file creates/edits locally but runs terminal commands remotely. See a dev container use case for example. This is also quite different from what jetbrains expects using their remote ssh support. This PR might work fine in both tools but I'm not sure without testing. My take so far has been to install the entire extension in the remote host for vscode. IM not a huge jetbrains user (tho we have many in our organization). |
@chezsmithy thanks! @etherandrius what was the original intent for this PR, does it fix a specific problem with commands being run remotely when they shouldn't for a given |
Description
Enabling the terminal command for all remotes; I'm pretty sure this is no longer needed?
The fallback says it's not enabled because it does not work for SSH but at the same time the feature is explicitly enabled for
ssh-remote
See definition of
vscode.env.remoteName
Summary by cubic
Enables terminal command execution across all environments by removing remoteName gating and using a single spawn-based path. SSH, WSL, Dev Containers, Codespaces, and local now get live output, background support, and clear status messages.
New Features
Refactors