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.

Sign up for GitHub

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

Jump to bottom

Add Julia as Shell type #225083

Merged
merged 6 commits into from
Aug 9, 2024
Merged

Add Julia as Shell type #225083

merged 6 commits into from
Aug 9, 2024

Conversation

anthonykim1
Copy link
Contributor

@anthonykim1 anthonykim1 commented Aug 7, 2024 •
edited
Loading

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, 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
1chooo
1chooo approved these changes Aug 9, 2024
View reviewed changes
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:

vscode/src/vs/platform/terminal/node/terminalProcess.ts

Lines 407 to 416 in 559107b

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) });
}

vscode/src/vs/platform/terminal/node/windowsShellHelper.ts

Lines 115 to 134 in 559107b

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
6 checks passed
@anthonykim1 anthonykim1 mentioned this pull request Sep 5, 2024
Merged
@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.
Reviewers

@Tyriar Tyriar Tyriar approved these changes

@1chooo 1chooo 1chooo approved these changes

Assignees

@anthonykim1 anthonykim1

Labels
terminal-shell-integration Shell integration, command decorations, etc.
Projects
None yet
Milestone
August 2024
Development

Successfully merging this pull request may close these issues.

Add Julia as a shell type in core
4 participants
@anthonykim1 @Tyriar @pfitzseb @1chooo