-
Notifications
You must be signed in to change notification settings - Fork 677
Centralize the logic about wsl #55
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
Conversation
Hi @purocean,
As we have discussed before. We can have some further improvement about the WSL support.
During my own experience, the main problems of current implementation are:
- WSL related logics separate in several different files
- The leetCodeBinaryPath will be wrong if the setting of
enableWSL
is switched by users. - We are using IO blocking method (execFileSync) to parse the WSL path.
So this is the reason I create a new class: leetCodeExecutor
, and try to put WSL related logic into this class as much as possible.
Very appreciated if you can help review this PR if you have time. Since this feature is originally contributed by you. 😄
@jdneo OK, I'll take a look.
@jdneo
Have you tested with enable useWsl
?
LeetCode extension need Node.js installed in environment path.
It occurs an error in my machine.
https://github.com/jdneo/vscode-leetcode/blob/8c52f42f37bcc48b0f79210545169e629ec456d3/src/utils/nodeUtils.ts#L10
https://github.com/jdneo/vscode-leetcode/blob/8c52f42f37bcc48b0f79210545169e629ec456d3/src/utils/cpUtils.ts#L11
Looks like have no specific treatment for WSL.
@purocean Nice finding! I'll fix it.
@purocean Updated. Please have a try when you have time.
@jdneo
Work fine for me.
Just a little wrong text.
Submitting to LeetCode
@purocean Emm, I'm sorry. Anything wrong with "Submitting to LeetCode..."?
@jdneo
The word submit
makes me confused. 😂 But not a big deal.
https://github.com/jdneo/vscode-leetcode/blob/526cedd1eb52c6383fe1342c9d3b744bde784b39/src/leetCodeExecutor.ts#L101
Haha, got it.
We can revisit the wording issue if we get user feedbacks in the future.
Uh oh!
There was an error while loading. Please reload this page.
fix #48
fix #54