-
Notifications
You must be signed in to change notification settings - Fork 22
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
Tweak how the terminal is launched on Windows #814
Tweak how the terminal is launched on Windows #814
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend viewing this diff with the Hide whitespace
option enabled on GitHub, since a large part of my changes here is just re-indentation.
I tested it on my Windows machine and confirmed that with the build from this PR, However, everything still worked fine when I tested the same with 1.3.1-beta1. 🤔 |
I can confirm that in 1.3.2, Studio picks up the user-level |
I can't reproduce it in v1.0.5, either… I'm beginning to wonder if this was only reproducible while running the development version of Studio to begin with |
@fredrikekelund do you mean development build? Or the app executed via |
I meant when the app is started with
Interesting. Thanks for sharing that, @wojtekn. |
Here's a quick status update. The tl;dr is that I've changed the scope of this PR to fix more cosmetic issues with how the terminal is launched on Windows.
|
I tested the new build:
|
Related issues
Proposed Changes
Window distinguishes between user-level and system-level environment variables. Studio launches with the system-levelPATH
environment variable, which is then inherited by the command prompt when users click theOpen in… Terminal
button in Studio (this is just how Node'schild_process
module works).This can be problematic for users because launching a new command prompt from the Start menu behaves differently. In that case, the user-levelPATH
environment variable is used. Starting a terminal with an unexpectedPATH
can obviously be troublesome since you may be missing the utilities you're used to.This PR changes the logic for how the terminal is launched on Windows to fix this issue. The new logic depends on PowerShell. AFAIU, PowerShell comes bundled with all currently supported Windows versions, so this PR should hopefully work for essentially all of our users.After more testing, we concluded that we can't reliably reproduce this issue (see #814 (comment)). I repurposed this PR to focus on more cosmetic issues with how the terminal is launched on Windows
Testing Instructions
PATH
envvar contains the WP-CLI path, but the system-levelPATH
does not (see clarification)Open in… Terminal
buttonwp
to ensure that the default, user-levelPATH
envvar is loadedPre-merge Checklist