KEMBAR78
add `onTerminal` activation event by meganrogge · Pull Request #254444 · microsoft/vscode · GitHub
Skip to content

Conversation

@meganrogge
Copy link
Contributor

fix #252413

@meganrogge meganrogge added this to the July 2025 milestone Jul 7, 2025
@meganrogge meganrogge self-assigned this Jul 7, 2025
@meganrogge meganrogge requested a review from Tyriar July 7, 2025 14:11
Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com>
Tyriar
Tyriar previously approved these changes Jul 7, 2025
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.

Let's merge after we get the ok in the API sync.

Tyriar
Tyriar previously requested changes Jul 8, 2025
@meganrogge meganrogge enabled auto-merge (squash) July 8, 2025 19:10
@meganrogge meganrogge requested a review from Tyriar July 8, 2025 20:31
@meganrogge meganrogge merged commit aed497c into main Jul 8, 2025
19 checks passed
@meganrogge meganrogge deleted the merogge/on-terminal branch July 8, 2025 23:17
Copy link

@kike7laguna kike7laguna left a comment

Choose a reason for hiding this comment

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

Verifica

Comment on lines +1027 to +1031
this._register(instance.onDidChangeShellType(() => this._extensionService.activateByEvent(`onTerminal:${instance.shellType}`)));
if (instance.shellType) {
this._extensionService.activateByEvent(`onTerminal:${instance.shellType}`);
}
return instance;
Copy link
Member

Choose a reason for hiding this comment

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

You can't register instance listeners against terminalservice like this as the references will remain when the terminal instance is disposed, potentially holding onto the TerminalInstance reference. You should do this in _initInstanceListeners

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.

onTerminal activation event

5 participants