support WSL #46
support WSL #46
Conversation
|
@purocean Thank you for the contribution. I'll take a look at this PR these days. |
src/utils/wslUtils.ts
Outdated
| } | ||
|
|
||
| export function toWinPath(path: string): string { | ||
| return cp.execFileSync("wsl", ["--", "wslpath", "-w", path]).toString().trim(); |
There was a problem hiding this comment.
It looks the 1st param "--" doesn't work for me.
CMD>wsl wslpath -w /mnt/c/
C:\CMD>wsl -- wslpath -w /mnt/c/
/bin/bash: --: invalid option
Usage: /bin/bash [GNU long option] [option] ...
/bin/bash [GNU long option] [option] script-file ...
GNU long options:
--debug
--debugger
--dump-po-strings
--dump-strings
--help
--init-file
--login
--noediting
--noprofile
--norc
--posix
--rcfile
--restricted
--verbose
--version
Shell options:
-ilrsD or -c command or -O shopt_option (invocation only)
-abefhkmnptuvxBCHP or -o optionThere was a problem hiding this comment.
Please check the Windows version, maybe this param can be removed.
C:\>ver
Microsoft Windows [Version 10.0.17713.1000]
C:\>wsl --help
Usage: wsl.exe [option] ...
Options:
-d, --distribution <DistributionName>
Launch the specified distribition.
-e, --exec <CommandLine>
Execute the specified Linux command. The remainder of the arguments are
used as the command line to execute.
-u, --user <UserName>
Run as the specified user.
--help
Display this usage information.
--
Stop parsing arguments and pass the remainder to the Linux process.
C:\>wsl -- wslpath -u "C:/"
/mnt/c/
There was a problem hiding this comment.
Attaching mine.
C:\>ver
Microsoft Windows [Version 10.0.17134.167]
C:\>wsl --help
/bin/bash: --: invalid option
Usage: /bin/bash [GNU long option] [option] ...
/bin/bash [GNU long option] [option] script-file ...
GNU long options:
--debug
--debugger
--dump-po-strings
--dump-strings
--help
--init-file
--login
--noediting
--noprofile
--norc
--posix
--rcfile
--restricted
--verbose
--version
Shell options:
-ilrsD or -c command or -O shopt_option (invocation only)
-abefhkmnptuvxBCHP or -o option
There was a problem hiding this comment.
I think wsl.exe is a wrapper to run inner shell in your WSL. I suppose the parameters depend on what your inner shell is.
There was a problem hiding this comment.
@purocean If you remove the -- at your side, will this command still workable?
There was a problem hiding this comment.
https://docs.microsoft.com/en-us/windows/wsl/release-notes#wsl-5
@jdneo yes, perfect. So we can remove it.
package.json
Outdated
| "type": "boolean", | ||
| "default": false, | ||
| "scope": "window", | ||
| "description": "Use nodejs inside the Windows Subsystem for Linux." |
|
Looks like there are several places which contains the logic like I think it could be better to have a pathUtils that wrap this kind of logic there. @purocean what do you think? |
| } | ||
|
|
||
| export function toWslPath(path: string): string { | ||
| return cp.execFileSync("wsl", ["wslpath", "-u", `${path.replace(/\\/g, "/")}`]).toString().trim(); |
There was a problem hiding this comment.
If we use " to wrap the path instead of replace \, will it be workable?
There was a problem hiding this comment.
Not Work :(
function toWslPath(path) {
return cp.execFileSync("wsl", ["wslpath", "-u", `"${path}"`]).toString().trim();
}module.js:549
throw err;
^
Error: Cannot find module '/mnt/y/env/VSCode-win32-x64/Y:/env/VSCode-win32-x64/.vscode/extensions/shengchen.vscode-leetcode-0.7.0/node_modules/leetcode-cli/bin/leetcode'
at Function.Module._resolveFilename (module.js:547:15)
at Function.Module._load (module.js:474:25)
at Function.Module.runMain (module.js:693:10)
at startup (bootstrap_node.js:191:16)
at bootstrap_node.js:612:3
There was a problem hiding this comment.
With pathUtils my point of view is not unnecessary at this time.
For this program, we just need do good job with file system and shell command for cross-platform. I think the cpUtils and workspaceUtils is sufficiently abstract.
We can try to change these next time if needed.
There was a problem hiding this comment.
We could focus on cpUtils and workspaceUtils provide service for others.
There was a problem hiding this comment.
@purocean That is because by default the option shell of execFileSync is false. If add {shell: true} to the argument list, then I think it should work.
Another thing I'm concerning is that here we use execFileSync(). Personally, I prefer use async/await wherever possible. Take a look at this: https://nodejs.org/en/docs/guides/blocking-vs-non-blocking/
So can we use executeCommand in the cpUtils?
There was a problem hiding this comment.
{shell: true} work perfectly.
We need a lot of works if we use async/await. Abstract leetcode command is a better option.
But we could improve step by step. Half a loaf is better than no bread.
There was a problem hiding this comment.
OK, make sense. Tracking issue: #48
|
@purocean Merged, thank you. |
resolve #47