Skip to content

Enable shell integration in hidden terminals #208036

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.

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

Merged
merged 1 commit into from
Mar 18, 2024
Merged

Enable shell integration in hidden terminals #208036

merged 1 commit into from
Mar 18, 2024

Conversation

Tyriar
Copy link
Member

@Tyriar Tyriar commented Mar 18, 2024

As I understand it, hidden terminals were always exempty from shell integration because hidden terminals were thought of as private to an extension in a way, and also since they probably weren't going to be interacted with. Especially since the introduction of the environment variable collection API I think we should change this decision, not doing so will lead to weirdness for extensions where hidden ones (which may be shown later) aren't fully featured and get inconsistent envs.

Fixes #199611
Part of #197187

A hideFromUser terminal after showing:

Screenshot 2024-03-18 at 11 54 23 AM

As I understand it, hidden terminals were always exempty from shell
integration because hidden terminals were thought of as private to an
extension in a way, and also since they probably weren't going to be
interacted with. Especially since the introduction of the environment
variable collection API I think we should change this decision, not
doing so will lead to weirdness for extensions where hidden ones (which
may be shown later) aren't fully featured and get inconsistent envs.

Fixes #199611
Part of #197187
@Tyriar Tyriar added this to the March 2024 milestone Mar 18, 2024
@Tyriar Tyriar requested a review from karrtikr March 18, 2024 19:02
@Tyriar Tyriar self-assigned this Mar 18, 2024
@Tyriar Tyriar enabled auto-merge March 18, 2024 19:02
Copy link
Contributor

@karrtikr karrtikr left a comment

Choose a reason for hiding this comment

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

LGTM as long as we can find a way for: #194819 (comment).

@Tyriar Tyriar merged commit b3701a9 into main Mar 18, 2024
@Tyriar Tyriar deleted the tyriar/199611 branch March 18, 2024 19:57
@microsoft microsoft locked and limited conversation to collaborators Jun 10, 2024
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.

onDidExecuteTerminalCommand does not fire events for hidden terminal
2 participants