Skip to content

Add onDidChangeShell event to the API#160900

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

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

Conversation

@babakks
Copy link
Contributor

@babakks babakks commented Sep 14, 2022

Fixes #160694

@babakks babakks changed the title Add change shell event Add onDidChangeShell event to the API Sep 14, 2022
Copy link
Member

@Tyriar Tyriar left a comment

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 on lines +9181 to +9185
/**
* An {@link Event} which fires when the default shell changes.
*/
export const onDidChangeShell: Event<string>;

Copy link
Member

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
Copy link
Contributor Author

babakks commented Sep 14, 2022

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

Tyriar
Tyriar previously approved these changes Sep 14, 2022
Copy link
Member

@Tyriar Tyriar left a comment

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
Copy link
Member

Tyriar commented Sep 14, 2022

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.

lramos15
lramos15 previously approved these changes Sep 14, 2022
@babakks
Copy link
Contributor Author

babakks commented Sep 14, 2022

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