Skip to content

Commit

Permalink
Meta tweaks
Browse files Browse the repository at this point in the history
  • Loading branch information
sindresorhus committed Oct 15, 2019
1 parent 7ef15d2 commit d13bbe3
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 21 deletions.
13 changes: 8 additions & 5 deletions index.js
Expand Up @@ -8,7 +8,7 @@ const isWsl = require('is-wsl');
const pAccess = promisify(fs.access);
const pExecFile = promisify(childProcess.execFile);

// Path to included `xdg-open`
// Path to included `xdg-open`.
const localXdgOpenPath = path.join(__dirname, 'xdg-open');

// Convert a path from WSL format to Windows format:
Expand Down Expand Up @@ -41,8 +41,8 @@ module.exports = async (target, options) => {
}

// Encodes the target as if it were an URL. Especially useful to get
// double-quotes through the double-quotes on windows caveat, but it
// can be used in any platform.
// double-quotes through the double-quotes on Windows caveat, but it
// can be used on any platform.
if (options.url) {
target = encodeURI(target);
}
Expand All @@ -68,7 +68,9 @@ module.exports = async (target, options) => {
// Always quoting target allows for URLs/paths to have spaces and unmarked characters, as `cmd.exe` will
// interpret them as plain text to be forwarded as one unique argument. Enabling `windowsVerbatimArguments`
// disables Node.js's default quotes and escapes handling (https://git.io/fjdem).
// References: Issues #17, #44, #55, #77, #101 and #115 / Pull requests: #74 and #98
// References:
// - Issues #17, #44, #55, #77, #101, #115
// - Pull requests: #74, #98
//
// As a result, all double-quotes are stripped from the `target` and do not get to your desired destination.
target = `"${target}"`;
Expand All @@ -81,6 +83,7 @@ module.exports = async (target, options) => {
if (options.app) {
if (isWsl && options.app.startsWith('/mnt/')) {
const windowsPath = await wslToWindowsPath(options.app);
// eslint-disable-next-line require-atomic-updates
options.app = windowsPath;
}

Expand All @@ -102,7 +105,7 @@ module.exports = async (target, options) => {
try {
await pAccess(localXdgOpenPath, fs.constants.X_OK);
exeLocalXdgOpen = true;
} catch (error) {}
} catch (_) {}

const useSystemXdgOpen = process.versions.electron ||
process.platform === 'android' || isBundled || !exeLocalXdgOpen;
Expand Down
4 changes: 2 additions & 2 deletions package.json
Expand Up @@ -52,7 +52,7 @@
"devDependencies": {
"@types/node": "^12.7.5",
"ava": "^2.3.0",
"tsd": "^0.7.4",
"xo": "^0.24.0"
"tsd": "^0.9.0",
"xo": "^0.25.3"
}
}
7 changes: 5 additions & 2 deletions readme.md
Expand Up @@ -103,14 +103,16 @@ We do not recommend using it on targets that are not URLs.

Especially useful when dealing with the [double-quotes on Windows](#double-quotes-on-windows) caveat.


## Caveats

### Double-quotes on Windows

TL;DR: All double-quotes are stripped from the `target` and do not get to your desired destination (on Windows!).

Due to specific behaviors of Window's Command Prompt (`cmd.exe`) regarding ampersand (`&`) characters breaking commands and URLs, double-quotes are now a special case.

The solution ([#146](https://github.com/sindresorhus/open/pull/146)) to this and other problems was to leverage the fact that `cmd.exe` interprets a double-quoted argument as a plain text argument just by quoting it (like Node already does). Unfortunatelly `cmd.exe` can only do **one** of two things: handle them all **OR** not handle them at all. As per its own documentation:
The solution ([#146](https://github.com/sindresorhus/open/pull/146)) to this and other problems was to leverage the fact that `cmd.exe` interprets a double-quoted argument as a plain text argument just by quoting it (like Node.js already does). Unfortunately, `cmd.exe` can only do **one** of two things: handle them all **OR** not handle them at all. As per its own documentation:

>*If /C or /K is specified, then the remainder of the command line after the switch is processed as a command line, where the following logic is used to process quote (") characters:*
>
Expand All @@ -123,10 +125,11 @@ The solution ([#146](https://github.com/sindresorhus/open/pull/146)) to this and
>
> 2. *Otherwise, old behavior is to see if the first character is a quote character and if so, strip the leading character and remove the last quote character on the command line, preserving any text after the last quote character.*
The option that solved all of the problems was the second one, and for additional behavior consistency we're also now using the `/S` switch, so we **always** get the second option. The caveat is that this built-in double-quotes handling ends up stripping all of them from the command line and so far we weren't able to find a scaping method that works (if you do, please feel free to contribute!).
The option that solved all of the problems was the second one, and for additional behavior consistency we're also now using the `/S` switch, so we **always** get the second option. The caveat is that this built-in double-quotes handling ends up stripping all of them from the command line and so far we weren't able to find an escaping method that works (if you do, please feel free to contribute!).

To make this caveat somewhat less impactful (at least for URLs), check out the [url option](#url). Double-quotes will be "preserved" when using it with an URL.


## Related

- [open-cli](https://github.com/sindresorhus/open-cli) - CLI for this module
Expand Down
24 changes: 12 additions & 12 deletions test.js
Expand Up @@ -33,19 +33,19 @@ test('wait for the app to close if wait: true', async () => {
await open('https://sindresorhus.com', {wait: true});
});

test('encode url if url: true', async () => {
test('encode URL if url: true', async () => {
await open('https://sindresorhus.com', {url: true});
});

test('open url in default app', async () => {
test('open URL in default app', async () => {
await open('https://sindresorhus.com');
});

test('open url in specified app', async () => {
test('open URL in specified app', async () => {
await open('https://sindresorhus.com', {app: firefoxName});
});

test('open url in specified app with arguments', async () => {
test('open URL in specified app with arguments', async () => {
await open('https://sindresorhus.com', {app: [chromeName, '--incognito']});
});

Expand All @@ -54,36 +54,36 @@ test('return the child process when called', async t => {
t.true('stdout' in cp);
});

test('open url with query strings', async () => {
test('open URL with query strings', async () => {
await open('https://sindresorhus.com/?abc=123&def=456');
});

test('open url with a fragment', async () => {
test('open URL with a fragment', async () => {
await open('https://sindresorhus.com#projects');
});

test('open url with query strings and spaces', async () => {
test('open URL with query strings and spaces', async () => {
await open('https://sindresorhus.com/?abc=123&def=456&ghi=with spaces');
});

test('open url with query strings and a fragment', async () => {
test('open URL with query strings and a fragment', async () => {
await open('https://sindresorhus.com/?abc=123&def=456#projects');
});

test('open url with query strings and pipes', async () => {
test('open URL with query strings and pipes', async () => {
await open('https://sindresorhus.com/?abc=123&def=456&ghi=w|i|t|h');
});

test('open url with query strings, spaces, pipes and a fragment', async () => {
test('open URL with query strings, spaces, pipes and a fragment', async () => {
await open('https://sindresorhus.com/?abc=123&def=456&ghi=w|i|t|h spaces#projects');
});

if (isWsl) {
test('open url in specified windows app given a wsl path to the app', async () => {
test('open URL in specified windows app given a wsl path to the app', async () => {
await open('https://sindresorhus.com', {app: firefoxWslName});
});

test('open url in specified windows app with arguments given a wsl path to the app', async () => {
test('open URL in specified windows app with arguments given a wsl path to the app', async () => {
await open('https://sindresorhus.com', {app: [chromeWslName, '--incognito']});
});
}

0 comments on commit d13bbe3

Please sign in to comment.