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

API to access the shell type for a given terminal #230165

Open
karthiknadig opened this issue Sep 30, 2024 · 10 comments · May be fixed by #237624
Open

API to access the shell type for a given terminal #230165

karthiknadig opened this issue Sep 30, 2024 · 10 comments · May be fixed by #237624
Assignees
Labels
api api-proposal feature-request Request for new features or functionality terminal General terminal issues that don't fall under another label
Milestone

Comments

@karthiknadig
Copy link
Member

Currently there is no API available on the terminal that allows extensions to detect the shell type. This is needed for Python where there are shell specific activation scripts.

@karthiknadig karthiknadig added the feature-request Request for new features or functionality label Sep 30, 2024
@meganrogge meganrogge added the api label Sep 30, 2024
@gjsjohnmurray
Copy link
Contributor

Does #127374 (comment) point to a way?

@karthiknadig
Copy link
Member Author

@gjsjohnmurray That is not available to Terminals created using ExtensionTerminalOptions.

@gjsjohnmurray
Copy link
Contributor

There isn't a universal concept of "shell type" for a terminal contributed by an extension. However it's possible to define your own interface that extends ExtensionTerminalOptions so it can hold other properties that get set when the extension terminal gets created. These extra properties will be present in Terminal.creationOptions when a terminal is fetched from window.activeTerminal or window.terminals[]

@karthiknadig
Copy link
Member Author

@gjsjohnmurray There is a terminalShellType context that is set for terminals, it should likely be set as a property on the terminal itself.

export type TerminalShellType = PosixShellType | WindowsShellType | GeneralShellType;

export const shellType = new RawContextKey<string>(TerminalContextKeyStrings.ShellType, undefined, { type: 'string', description: localize('terminalShellTypeContextKey', "The shell type of the active terminal, this is set if the type can be detected.") });

The issue is that, when any extension or VS Code creates a terminal (DAP run-in-terminal for example) or the user click on the ➕ button to create terminals there is no consistent way to detect what shell it is using by examining the terminal instance. I understand that the shell type is a transient thing in a terminal. But, to provide better experience we need to know what it is at that moment for the terminal.

For Python extension, we need this for terminals created from any source, not just from python extension. The reason we need it is so we can configure the terminal to use the right python and packages from the correct locations. Typically, this is done by running a script provided by the python environment. These scripts are shell specific, and without this detail it leads to a bad terminal experience if we can't configure the environment correctly.

Currently we detect shells is by using the Terminal.name on terminal creation. This is limiting because, when some other source like DAP creates it that name is not set to the shell type, and that breaks detection. In cases where user clicks the ➕ the name is set to shell type.

@Tyriar
Copy link
Member

Tyriar commented Oct 4, 2024

@gjsjohnmurray we have some detection internally based on various factors like process name, executables in the process tree, etc. Exposing that would work for terminals created via extensions too, as long as they are backed by an executable, not a Pseudoterminal API object.

@Tyriar Tyriar added this to the November 2024 milestone Oct 16, 2024
@Tyriar
Copy link
Member

Tyriar commented Oct 28, 2024

Proposal:

declare module 'vscode' {

	// https://github.com/microsoft/vscode/issues/230165

	/**
	 * Known terminal shell types.
	 */
	export enum TerminalShellType {
		Sh = 1,
		CommandPrompt,
		PowerShell,
		GitBash,
		Bash,
		Zsh,
		Python,
		Julia,
		NuShell,
		Fish,
		Csh,
		Ksh
	}

	// NOTE: State since this the shellType can change multiple times and this comes with an event.
	export interface TerminalState {
		/**
		 * The current detected shell type of the terminal. New shell types may be added in the
		 * future in which case they will be returned as a number that is not part of
		 * {@link TerminalShellType}.
		 * TODO: number is to prevent the breaking change when new enum members are added?
		 */
		readonly shellType: TerminalShellType | number | undefined;
	}

}

@karthiknadig any feedback?

@Tyriar
Copy link
Member

Tyriar commented Oct 28, 2024

The TerminalShellType | number approach seems ok for now as a means to prevent breaking in an enum that's likely changing in the future. Will probably get discussed more during finalization.

@Tyriar
Copy link
Member

Tyriar commented Oct 28, 2024

We should consider setting shellType based on the initial shell executable path so it's non-undefined initially.

@Tyriar
Copy link
Member

Tyriar commented Oct 28, 2024

More feedback, if we run Python via WASM it won't currently get the shell type. Should we do something like this?

	interface Pseudoterminal {
		shellType?: TerminalShellType;

@Tyriar
Copy link
Member

Tyriar commented Nov 11, 2024

Added @anthonykim1 since he's done a bunch with shell types at this point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api api-proposal feature-request Request for new features or functionality terminal General terminal issues that don't fall under another label
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants