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

[karrtikr]

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]

same problem

[Tyriar]

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

[Tyriar]

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

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

[anthonykim1]

@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]

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]

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

vscode/src/vs/workbench/api/browser/mainThreadTerminalShellIntegration.ts

Lines 37 to 44 in 5a68b64

	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):

vscode/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts

Lines 334 to 394 in 5a68b64

	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:

vscode/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts

Lines 334 to 394 in 5a68b64

	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