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.

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

env.shell API should not be the empty string on start up #190974

Open
Tyriar opened this issue Aug 22, 2023 · 3 comments
Open

env.shell API should not be the empty string on start up #190974

Tyriar opened this issue Aug 22, 2023 · 3 comments
Assignees
Labels
api bug Issue identified by VS Code Team member as probable bug terminal-process Problems launching processes, managing ptys, exiting, process leaks, etc.
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Aug 22, 2023

Currently env.shell always starts as undefined, since the cost to evaluate it is 50-100ms we don't want to do this before the extension host launches. We could cache the value optimistically across sessions though and in the off chance that it is wrong it will fire the upcoming event quickly.

Context: #160694 (comment)

@Tyriar Tyriar added api feature-request Request for new features or functionality terminal General terminal issues that don't fall under another label labels Aug 22, 2023
@Tyriar Tyriar added this to the August 2023 milestone Aug 22, 2023
@Tyriar Tyriar self-assigned this Aug 22, 2023
@Tyriar Tyriar modified the milestones: August 2023, September 2023 Aug 23, 2023
@Tyriar
Copy link
Member Author

Tyriar commented Sep 7, 2023

Instead of caching we're going to run it on the main process after the window opens and before the ext host launches, simialr to resolveShellEnvironment.

@Tyriar Tyriar changed the title Cache env.shell API across sessions env.shell API should not be the empty string on start up Sep 12, 2023
@Tyriar Tyriar added bug Issue identified by VS Code Team member as probable bug and removed feature-request Request for new features or functionality labels Sep 12, 2023
@Tyriar
Copy link
Member Author

Tyriar commented Sep 12, 2023

Merging into #160694

@Tyriar Tyriar closed this as completed Sep 12, 2023
@Tyriar Tyriar added the *duplicate Issue identified as a duplicate of another issue(s) label Sep 12, 2023
@Tyriar Tyriar reopened this Sep 25, 2023
@Tyriar Tyriar removed the *duplicate Issue identified as a duplicate of another issue(s) label Sep 25, 2023
@Tyriar Tyriar modified the milestones: September 2023, October 2023 Sep 25, 2023
Tyriar added a commit that referenced this issue Sep 28, 2023
Part of #190974
@Tyriar
Copy link
Member Author

Tyriar commented Oct 23, 2023

I looked into this and it was a lot harder than I originally anticipated because currently info on the workspace and other context in the window is used in order to evaluate profiles, for example of this is in order to resolve variables.

@Tyriar Tyriar modified the milestones: October 2023, Backlog Oct 23, 2023
@Tyriar Tyriar added terminal-process Problems launching processes, managing ptys, exiting, process leaks, etc. and removed terminal General terminal issues that don't fall under another label labels Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api bug Issue identified by VS Code Team member as probable bug terminal-process Problems launching processes, managing ptys, exiting, process leaks, etc.
Projects
None yet
Development

No branches or pull requests

1 participant