Skip to content

Add onDidChangeShell event to the API#160900

Merged
Tyriar merged 9 commits into
microsoft:mainfrom
babakks:add-change-shell-event
Sep 14, 2022
Merged

Add onDidChangeShell event to the API#160900
Tyriar merged 9 commits into
microsoft:mainfrom
babakks:add-change-shell-event

Conversation

@babakks

@babakks babakks commented Sep 14, 2022

Copy link
Copy Markdown
Contributor

Fixes #160694

Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>
Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>
@babakks babakks changed the title Add change shell event Add onDidChangeShell event to the API Sep 14, 2022

@Tyriar Tyriar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks! Couple of comments and this looks good to go

Comment thread src/vs/workbench/api/common/extHostTerminalService.ts
Comment thread src/vscode-dts/vscode.d.ts Outdated
Comment on lines +9181 to +9185
/**
* An {@link Event} which fires when the default shell changes.
*/
export const onDidChangeShell: Event<string>;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This needs to move into a vscode.proposed.envShellEvent.d.ts file instead to go through our proposal/finalization process.

@Tyriar Tyriar added this to the September 2022 milestone Sep 14, 2022
@Tyriar Tyriar self-assigned this Sep 14, 2022
babakks and others added 3 commits September 14, 2022 18:17
Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com>
Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>
Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>
@babakks

babakks commented Sep 14, 2022

Copy link
Copy Markdown
Contributor Author

@Tyriar If you're okay, I can rebase the commits to avoid garbage commits on the main branch.

Comment thread src/vs/workbench/api/common/extHostTerminalService.ts
Tyriar
Tyriar previously approved these changes Sep 14, 2022

@Tyriar Tyriar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @babakks, going to merge this in and will bring it to the next (internal) API sync

@Tyriar

Tyriar commented Sep 14, 2022

Copy link
Copy Markdown
Contributor

If you're okay, I can rebase the commits to avoid garbage commits on the main branch.

It's fine as they're all linked to the merge commit, the merger can squash or rebase before merging if they want thought the github UI as well.

Comment thread src/vs/workbench/api/common/extHost.api.impl.ts
Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>
Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>
Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>
lramos15
lramos15 previously approved these changes Sep 14, 2022
@babakks

babakks commented Sep 14, 2022

Copy link
Copy Markdown
Contributor Author

Sorry @Tyriar since you had it reviewed already, but had to improve the style for the event property in fca8afc.

Tyriar
Tyriar previously approved these changes Sep 14, 2022
@Tyriar
Tyriar enabled auto-merge September 14, 2022 14:24
@Tyriar
Tyriar dismissed stale reviews from lramos15 and themself via c461129 September 14, 2022 14:32
@Tyriar
Tyriar merged commit a4cce8f into microsoft:main Sep 14, 2022
@babakks
babakks deleted the add-change-shell-event branch September 14, 2022 14:56
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vscode.env.shell might return empty string to extension if fetched very soon after startup

3 participants