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

Support WSL configuration where Windows paths are not in PATH #195

Merged
merged 7 commits into from
Sep 29, 2020
Merged

Conversation

Pytal
Copy link
Contributor

@Pytal Pytal commented Sep 23, 2020

Fixes #173 with the caveat of the wait option being ignored as mentioned here.

Fixes #186

@sindresorhus
Copy link
Owner

Did you see #188 (comment) ?

// @tim-stasse

@Pytal
Copy link
Contributor Author

Pytal commented Sep 23, 2020

Yes, I referenced it at the top. I noted the caveat in index.d.ts.

@sindresorhus
Copy link
Owner

No, I meant that the existing implementation does the same as wslview does under the hood without the caveat, so what's the benefit of this PR?

@Pytal
Copy link
Contributor Author

Pytal commented Sep 23, 2020

Right sorry for the misunderstanding.

The use case for this is for users on WSL who do not have Windows paths exposed to WSL as seen in this create-react-app issue and therefore do not necessarily have powershell in PATH.

@sindresorhus
Copy link
Owner

The use case for this is for users on WSL who do not have Windows paths exposed to WSL as seen in this create-react-app issue and therefore do not necessarily have powershell in PATH.

And there's no way to fix this without using wslview? I would explore that option first.

@Pytal
Copy link
Contributor Author

Pytal commented Sep 24, 2020

I only know of wslview as a utility baked into WSL (on a few distros, most notably Ubuntu) for opening URLs and files in Windows. Do you have any suggestions for an alternative solution where neither wslview nor powershell is used?

Maybe a hardcoded path to powershell could be used when it can't be found in PATH, although this feels hacky and is probably unexpected bahviour for the end-user who has opted not to expose Windows paths.

@tim-stasse
Copy link
Contributor

@Pytal I believe the actual issue that needs to be resolved here is #186

As I mentioned in #188, wslview uses powershell.exe under the hood. It works regardless of whether powershell.exe is on the path—or not—by programmatically constructing the path to the windows installation folder, and then appending a hard coded path to the powershell binary from there—you can see what I mean by taking a look at lines 111, 125, and 154 from wslview's source code.

@Pytal
Copy link
Contributor Author

Pytal commented Sep 24, 2020

@tim-stasse Would the solution mentioned here suffice? Or is it preferable to not use another binary?

@tim-stasse
Copy link
Contributor

@Pytal wslvar comes from the same set of wslu binaries as wslview, so yes it should work.

Probably just something like the following should work:

command = '$(wslvar systemroot)\System32\WindowsPowerShell\v1.0\powershell.exe'
cliArguments.push(
	'-NoProfile',
	'-NonInteractive',
	'–ExecutionPolicy',
	'Bypass',
	'-EncodedCommand'
);

if (isWsl) {
	command = await wslToWindowsPath(command);
} else {
	childProcessOptions.windowsVerbatimArguments = true;
}

@Pytal Pytal changed the title Add support for wslview on WSL Support WSL configuration where Windows paths are not in PATH Sep 26, 2020
@Pytal
Copy link
Contributor Author

Pytal commented Sep 26, 2020

@tim-stasse I've made the changes using wslvar and I'd appreciate your feedback

@tim-stasse
Copy link
Contributor

@Pytal Looks good to me 👍

@sindresorhus sindresorhus merged commit be0f794 into sindresorhus:master Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cmd.exe is expected to be available on the PATH in WSL Alternative URL opener on WSL - wslview
3 participants