- Notifications
You must be signed in to change notification settings - Fork670
fixed use wsl feature (issue#509)#510
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
convert path wsl after full path constructedleetCodeRootPathInWsl no longer usedgetLeetCodeRootPath function removed since it has only one reference after the bug fixed
add missing signature
This pull requestintroduces 1 alert when mergingf3ad33f into1c4a39e -view on LGTM.com new alerts:
|
src/leetCodeExecutor.ts Outdated
| this.leetCodeRootPathInWsl = `${await wsl.toWslPath(this.leetCodeRootPath)}`; | ||
| } | ||
| return `${this.leetCodeRootPathInWsl}`; | ||
| return `${yield wsl.toWslPath(`"${path.join(yield`"${this.leetCodeRootPath}"`, "bin", "leetcode")}"`)}`; |
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.
No need to haveyield here
src/leetCodeExecutor.ts Outdated
| public async getLeetCodeBinaryPath(): Promise<string> { // wrapped by "" | ||
| return `"${path.join(await this.getLeetCodeRootPath(), "bin", "leetcode")}"`; | ||
| return `"${path.join(yield`"${this.leetCodeRootPath}"`, "bin", "leetcode")}"`; |
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.
No need to haveyield here
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.
Orz
i'm not too much familiar with ts, maybe you should take over this fix if i keep failed the build
patch to solve wslpath fail when path not exists
when open new problem, wslpath conversion to windows fails as the target path under wsl doesn't exists yet, I tried fixed this and append to this same PR |
Thank you @LinkeyLeo, Sorry I have no Windows machine right now (it's in my office, and given the current situation, It's not easy to get it). Could you please help me to understand the change? You have mentioned:
What's the path you mentioned here? |
the workflow for open problem for coding is looks like following: in step 3, we need to create a new source file for writing default codes, due to using node under wsl, the target file path reported by node is a wsl path, but the extension host is running under windows and need a windows path for creating and open the file, thus we need wslpath to convert the path, then the problem appeared. wslpath will finish the path conversion only if the path is valid and the path is exists, or it rise a path doesn't exist error and won't do the path conversion. considering other path may need to be conversion and have the same problem in the future, I fixed this problem by modified toWinPath function, making it only process the prefix of whole path and manually reconstruct the complete path |
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.
@LinkeyLeo My apologize for the late reply, sorry!
The PR looks good to me overall. Just one question, when will the path does not start with"\\mnt\\". we have this:return (await executeCommand("wsl", ["wslpath", "-w", "/"])).trim() + path;
Could you explain more about this?
Thanks.
LNKLEO commentedFeb 21, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
i divide wsl paths into two parts, mounted drive root from windows which located in /mnt/ and other path located in / except /mnt/ for the after ones, we need to map /{WSL PATH} to \wsl${distor name}{WSL PATH} |
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.
LGTM, thanks!
Uh oh!
There was an error while loading.Please reload this page.
convert path wsl after full path constructed
resolve problem cased by wslpath conversion
leetCodeRootPathInWsl no longer used
getLeetCodeRootPath function removed since it has only one reference after the bug fixed
#509