Skip to content
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

Resolves the issues with +x on npm #71

Closed
wants to merge 3 commits into from

Conversation

EvanCarroll
Copy link

Works again with linux, resolves the issue with npm disabling -x by sourcing the local xdg-open through /usr/bin/env

@EvanCarroll
Copy link
Author

This fixes #70


if (appArgs.length > 0) {
args = args.concat(appArgs);
cmd = path.join('/usr', 'bin', 'env');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd use something like which here to ensure that we don't use the wrong path.

if (appArgs.length > 0) {
args = args.concat(appArgs);
cmd = path.join('/usr', 'bin', 'env');
args.unshift('xdg-open');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not cmd = path.join('/usr', 'bin', 'env', 'xdg-open');?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because xdg-open is the argument to /usr/bin/env there is no app called /usr/bin/env/xdg-open Try running /usr/bin/env xdg-open "http://www.google.com", now try /usr/bin/env/xdg-open "http://www.google.com"

@@ -47,14 +51,15 @@ module.exports = (target, opts) => {
args = args.concat(appArgs);
}
} else {
if (appArgs.length > 0) {
args = args.concat(appArgs);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you move this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because unshifting (pushing to the front) of an array that isn't yet created is a bad idea. We'd otherwise get clobbered.

@sindresorhus
Copy link
Owner

Thanks, but I rather just wait on npm to fix this. Looks like a fix there is close: zkat/pacote#117

@EvanCarroll
Copy link
Author

That's no joy. Shipping out your own copy of xdg-open seems like a bad idea to begin with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants