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

Prepending using shell integration does not work for fish when using starship prompt #208465

Closed
Tracked by #22879
karrtikr opened this issue Mar 22, 2024 · 6 comments
Closed
Tracked by #22879
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug *out-of-scope Posted issue is not in scope of VS Code terminal-shell-fish An issue in the terminal specific to fish terminal-shell-integration Shell integration, command decorations, etc.

Comments

@karrtikr
Copy link
Contributor

karrtikr commented Mar 22, 2024

starship is a customized prompt. Same issue as #205133 (comment) but for fish.

Turns out #206994 was just fixing it for zsh, we should make sure to apply these changes to all shell integration scripts using VSCODE_SHELL_INTEGRATION.

cc/ @Tyriar

Originally reported in microsoft/vscode-python#23110 microsoft/vscode-python#22429

@kahosan
Copy link

kahosan commented Apr 15, 2024

same problem

@Tyriar
Copy link
Member

Tyriar commented Apr 15, 2024

Turns out just shell integration does not work at all when using starship under fish currently.

@Tyriar
Copy link
Member

Tyriar commented Apr 15, 2024

Shell integration overwrites the prompt completely which replaces our hooks that apply the env changes. To get shell integration working in this case you need to install it manually and disable the setting inside VS Code:

"terminal.integrated.shellIntegration.enabled": false

config.fish:

if status is-interactive
    starship init fish | source

    string match -q "$TERM_PROGRAM" "vscode"
        and . (code-insiders --locate-shell-integration-path fish)
end
Screenshot 2024-04-15 at 2 24 11 PM

The only way to work around this AFAIK is to write files to ~/.config/fish/conf.d/ which is something we don't want to do as it's overstepping our bounds and messing with a user's machine. The onDidChangeTerminalShellIntegration event should never fire when this by default so it should not block EnvironmentVariableCollection-based activation. cc @anthonykim1

@Tyriar Tyriar closed this as completed Apr 15, 2024
@Tyriar Tyriar added the *out-of-scope Posted issue is not in scope of VS Code label Apr 15, 2024
@Tyriar Tyriar reopened this Apr 17, 2024
@Tyriar Tyriar closed this as completed Apr 17, 2024
@rzhao271 rzhao271 added *out-of-scope Posted issue is not in scope of VS Code bug Issue identified by VS Code Team member as probable bug and removed *out-of-scope Posted issue is not in scope of VS Code bug Issue identified by VS Code Team member as probable bug labels Apr 25, 2024
@rzhao271 rzhao271 removed this from the April 2024 milestone Apr 25, 2024
@anthonykim1
Copy link
Contributor

@Tyriar Thanks for the insight.
Just to be clear, am I correct that onDidChangeTerminalShellIntegration event would NOT fire GIVEN user hasn't opted-in for manual shell integration & has not modified their config.fish when using starship prompt?

Would onDidChangeTerminalShellIntegration fire when user goes from not having shell integration manually installed to when they do have it installed?

@anthonykim1
Copy link
Contributor

I'm not sure if I understand: Shell integration overwrites the prompt completely which replaces our hooks that apply the env changes. correctly.

Shouldn't we still attempt to apply environment variable collections like in https://github.com/microsoft/vscode/pull/206994/files for other shells like fish to have EnvironmentVariableCollection-based activation even in case shell integration is disabled?

@Tyriar
Copy link
Member

Tyriar commented May 3, 2024

@anthonykim1 onDidChangeTerminalShellIntegration fires when we have "command detection":

const onDidAddCommandDetection = this._store.add(this._terminalService.createOnInstanceEvent(instance => {
return Event.map(
Event.filter(instance.capabilities.onDidAddCapabilityType, e => {
return e === TerminalCapability.CommandDetection;
}), () => instance
);
})).event;
this._store.add(onDidAddCommandDetection(e => this._proxy.$shellIntegrationChange(e.instanceId)));

This happens when we get any 633; command sequence (not cwd):

case VSCodeOscPt.PromptStart:
this._createOrGetCommandDetection(this._terminal).handlePromptStart();
return true;
case VSCodeOscPt.CommandStart:
this._createOrGetCommandDetection(this._terminal).handleCommandStart();
return true;
case VSCodeOscPt.CommandExecuted:
this._createOrGetCommandDetection(this._terminal).handleCommandExecuted();
return true;
case VSCodeOscPt.CommandFinished: {
const arg0 = args[0];
const exitCode = arg0 !== undefined ? parseInt(arg0) : undefined;
this._createOrGetCommandDetection(this._terminal).handleCommandFinished(exitCode);
return true;
}
case VSCodeOscPt.CommandLine: {
const arg0 = args[0];
const arg1 = args[1];
let commandLine: string;
if (arg0 !== undefined) {
commandLine = deserializeMessage(arg0);
} else {
commandLine = '';
}
this._createOrGetCommandDetection(this._terminal).setCommandLine(commandLine, arg1 === this._nonce);
return true;
}
case VSCodeOscPt.ContinuationStart: {
this._createOrGetCommandDetection(this._terminal).handleContinuationStart();
return true;
}
case VSCodeOscPt.ContinuationEnd: {
this._createOrGetCommandDetection(this._terminal).handleContinuationEnd();
return true;
}
case VSCodeOscPt.RightPromptStart: {
this._createOrGetCommandDetection(this._terminal).handleRightPromptStart();
return true;
}
case VSCodeOscPt.RightPromptEnd: {
this._createOrGetCommandDetection(this._terminal).handleRightPromptEnd();
return true;
}
case VSCodeOscPt.Property: {
const arg0 = args[0];
const deserialized = arg0 !== undefined ? deserializeMessage(arg0) : '';
const { key, value } = parseKeyValueAssignment(deserialized);
if (value === undefined) {
return true;
}
switch (key) {
case 'ContinuationPrompt': {
this._updateContinuationPrompt(removeAnsiEscapeCodesFromPrompt(value));
return true;
}
case 'Cwd': {
this._updateCwd(value);
return true;
}
case 'IsWindows': {
this._createOrGetCommandDetection(this._terminal).setIsWindowsPty(value === 'True' ? true : false);

Or 133; sequence:

case VSCodeOscPt.PromptStart:
this._createOrGetCommandDetection(this._terminal).handlePromptStart();
return true;
case VSCodeOscPt.CommandStart:
this._createOrGetCommandDetection(this._terminal).handleCommandStart();
return true;
case VSCodeOscPt.CommandExecuted:
this._createOrGetCommandDetection(this._terminal).handleCommandExecuted();
return true;
case VSCodeOscPt.CommandFinished: {
const arg0 = args[0];
const exitCode = arg0 !== undefined ? parseInt(arg0) : undefined;
this._createOrGetCommandDetection(this._terminal).handleCommandFinished(exitCode);
return true;
}
case VSCodeOscPt.CommandLine: {
const arg0 = args[0];
const arg1 = args[1];
let commandLine: string;
if (arg0 !== undefined) {
commandLine = deserializeMessage(arg0);
} else {
commandLine = '';
}
this._createOrGetCommandDetection(this._terminal).setCommandLine(commandLine, arg1 === this._nonce);
return true;
}
case VSCodeOscPt.ContinuationStart: {
this._createOrGetCommandDetection(this._terminal).handleContinuationStart();
return true;
}
case VSCodeOscPt.ContinuationEnd: {
this._createOrGetCommandDetection(this._terminal).handleContinuationEnd();
return true;
}
case VSCodeOscPt.RightPromptStart: {
this._createOrGetCommandDetection(this._terminal).handleRightPromptStart();
return true;
}
case VSCodeOscPt.RightPromptEnd: {
this._createOrGetCommandDetection(this._terminal).handleRightPromptEnd();
return true;
}
case VSCodeOscPt.Property: {
const arg0 = args[0];
const deserialized = arg0 !== undefined ? deserializeMessage(arg0) : '';
const { key, value } = parseKeyValueAssignment(deserialized);
if (value === undefined) {
return true;
}
switch (key) {
case 'ContinuationPrompt': {
this._updateContinuationPrompt(removeAnsiEscapeCodesFromPrompt(value));
return true;
}
case 'Cwd': {
this._updateCwd(value);
return true;
}
case 'IsWindows': {
this._createOrGetCommandDetection(this._terminal).setIsWindowsPty(value === 'True' ? true : false);

Would onDidChangeTerminalShellIntegration fire when user goes from not having shell integration manually installed to when they do have it installed?

onDidChangeTerminalShellIntegration fires per terminal after it launches. It will fire when it's manually activated or when the automatic injection succeeds

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug *out-of-scope Posted issue is not in scope of VS Code terminal-shell-fish An issue in the terminal specific to fish terminal-shell-integration Shell integration, command decorations, etc.
Projects
None yet
Development

No branches or pull requests

5 participants