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

Please don't require injecting strings into the integrated terminal #1558

Closed
danopia opened this issue Jun 11, 2021 · 3 comments
Closed

Please don't require injecting strings into the integrated terminal #1558

danopia opened this issue Jun 11, 2021 · 3 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@danopia
Copy link

danopia commented Jun 11, 2021

Is your feature request related to a problem? Please describe.
I'm always frustrated when I see commands get entered in the VSCode terminal, executed, and then cleared from view. It feels like a sketchy thing to be doing. I see that the commands are to update PATH:

if (terminal.name.toLowerCase() === 'cmd') {
terminal.sendText(`set PATH=${gorootBin};%Path%`, true);
terminal.sendText('cls');
} else if (['powershell', 'pwsh'].includes(terminal.name.toLowerCase())) {
terminal.sendText(`$env:Path="${gorootBin};$env:Path"`, true);
terminal.sendText('clear');
} else if (terminal.name.toLowerCase() === 'fish') {
terminal.sendText(`set -gx PATH ${gorootBin} $PATH`);
terminal.sendText('clear');
} else if (['bash', 'sh', 'zsh', 'ksh'].includes(terminal.name.toLowerCase())) {
terminal.sendText(`export PATH=${gorootBin}:$PATH`, true);
terminal.sendText('clear');
}

I've also noticed that if there is already text written into the shell, the automatic commands simply append to the partial command and a broken mashup of the commands are executed. Injecting enter keystrokes without a visible trigger is not safe.

My primary written language is not golang so I'd rather not have the golang extension act like it has ownership over my terminal.

Describe the solution you'd like
If the injection of PATH commands is absolutely necessary to provide a working out-of-box experience, I'd like to see a toggle to disable this behavior, or documentation on how to "fix" the environment to not require PATH changes.

I already have the correct go in my shell's environment and do not need the PATH change. vscode-go appears unaware of this.

Describe alternatives you've considered
The extension could instead provide an toast window which indicates that the go version is not in PATH, with a button to inject the commands. This way the user can close the dialog if they will not be using go in the session.

Any recurring dialog like this would also need to provide documentation (via link or otherwise) on how to correct the configuration so that future sessions will not need the command injected anymore.

Additional context
I'm using the code-server distribution of VSCode on macOS, and have it configured to be started as a service using launchctl:

~ $ launchctl list | grep -E 'brew|PID'
PID     Status  Label
540     0       homebrew.mxcl.code-server

Perhaps related to using code-server, I've noticed that only the initial terminal tab receives the command injection. So $PATH is different between my terminal tabs. The code in this extension seems to try to inject into every opened terminal so that might be perhaps to some different implementation of code-server terminals.

@gopherbot gopherbot added this to the Untriaged milestone Jun 11, 2021
@hyangah hyangah added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 11, 2021
@findleyr
Copy link
Contributor

We discussed this at our triage meeting today, and generally agree.

For now, we're going to add a configuration option to disable this functionality. Hopefully future releases of VS Code will have other ways to set the process environment for newly opened terminals.

@findleyr findleyr modified the milestones: Untriaged, On Deck Jun 11, 2021
@danopia
Copy link
Author

danopia commented Jul 13, 2021

I appreciate the vote of generally-agree :) I look forward to seeing a configuration option ship in the near feature 👍

@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/336409 mentions this issue: package.json: add go.terminal.activateEnvironment setting

@hyangah hyangah modified the milestones: On Deck, v0.27.0 Jul 22, 2021
@golang golang locked and limited conversation to collaborators Jul 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants