Skip to content

Conversation

@anthonykim1
Copy link
Contributor

@anthonykim1 anthonykim1 commented Aug 7, 2024

Adding Julia as Shell type.
Resolves: #224325

Will handle adding NuShell in separate PR
Will handle separating out pwsh, python, julia into GenericShellType Enum in separate PR.

@anthonykim1 anthonykim1 added the terminal-shell-integration Shell integration infrastructure, command decorations, etc. label Aug 7, 2024
@anthonykim1 anthonykim1 self-assigned this Aug 7, 2024
@anthonykim1 anthonykim1 added this to the August 2024 milestone Aug 8, 2024
@anthonykim1 anthonykim1 changed the title Draft) Start adding Julia as Shell type Add Julia as Shell type Aug 9, 2024
@anthonykim1 anthonykim1 marked this pull request as ready for review August 9, 2024 00:50
@anthonykim1 anthonykim1 requested a review from Tyriar August 9, 2024 00:50
Copy link

@1chooo 1chooo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look good to me!!!

@Tyriar
Copy link
Member

Tyriar commented Aug 9, 2024

FYI @davidanthoff, @pfitzseb

@pfitzseb
Copy link
Contributor

pfitzseb commented Aug 9, 2024

Oh, great to see! Is the shell type set based on the executable name (i.e. shellPath)?

@Tyriar
Copy link
Member

Tyriar commented Aug 9, 2024

@pfitzseb yes, but not necessarily the path that's passed in. There's some logic that inspects the process tree of the terminal to find the inner shell path/process name:

this._currentTitle = (ptyProcess.process ?? '');
this._onDidChangeProperty.fire({ type: ProcessPropertyType.Title, value: this._currentTitle });
// If fig is installed it may change the title of the process
const sanitizedTitle = this.currentTitle.replace(/ \(figterm\)$/g, '');
if (sanitizedTitle.toLowerCase().startsWith('python')) {
this._onDidChangeProperty.fire({ type: ProcessPropertyType.ShellType, value: PosixShellType.Python });
} else {
this._onDidChangeProperty.fire({ type: ProcessPropertyType.ShellType, value: posixShellTypeMap.get(sanitizedTitle) });
}

async getShellName(): Promise<string> {
if (this._store.isDisposed) {
return Promise.resolve('');
}
// Prevent multiple requests at once, instead return current request
if (this._currentRequest) {
return this._currentRequest;
}
if (!windowsProcessTree) {
windowsProcessTree = await import('@vscode/windows-process-tree');
}
this._currentRequest = new Promise<string>(resolve => {
windowsProcessTree.getProcessTree(this._rootProcessId, tree => {
const name = this.traverseTree(tree);
this._currentRequest = undefined;
resolve(name);
});
});
return this._currentRequest;
}

The idea being, if you launch bash inside pwsh, it should correctly get changed to bash.

@anthonykim1 anthonykim1 merged commit 9b803e9 into microsoft:main Aug 9, 2024
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Sep 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

terminal-shell-integration Shell integration infrastructure, command decorations, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Julia as a shell type in core

4 participants