Add support for Windows apps referenced by their WSL paths #118
Conversation
|
Sorry, let me fix these xo errors. |
index.js
Outdated
| // e.g. /mnt/c/Program Files/Example/MyApp.exe | ||
| // => C:\Program Files\Example\MyApp.exe | ||
| const wslToWindowsPath = async path => { | ||
| const {stdout} = await util.promisify(childProcess.execFile)('wslpath', ['-w', path]); |
There was a problem hiding this comment.
Can you use a long flag version of -w? For readability.
There was a problem hiding this comment.
Better to promisify at the top-level instead of every time it's called.
const pExecFile = util.promisify(childProcess.execFile);There was a problem hiding this comment.
Unfortunately wslpath doesn't have any long-form flags.
| @@ -43,6 +52,11 @@ module.exports = async (target, options) => { | |||
| } | |||
|
|
|||
| if (options.app) { | |||
| if (isWsl && options.app.startsWith('/mnt/')) { | |||
| const winPath = await wslToWindowsPath(options.app); | |||
| options.app = winPath; | |||
There was a problem hiding this comment.
| options.app = winPath; | |
| options.app = await wslToWindowsPath(options.app); |
There was a problem hiding this comment.
That fails atomic-assignment check since it reads options.app first and asynchronously reassigns to it, during which, theoretically, someone else can come to modify options.app and its modification will be lost. (This was discovered by xo).
|
Can you mention in the readme that it now correctly handles WSL mtn paths? |
|
@sindresorhus I think it's ready to be merged now? |
sindresorhus
changed the title
|
Yup. Looks good :) |
Fixes #117