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

Wrap the apm command in quotes in case it has spaces. #331

Merged

Conversation

deankevorkian
Copy link
Contributor

Fixes #330, as documented in Node.

Works on both my Windows and Mac machines, both with commands and paths to executables.

Copy link

@DeeDeeG DeeDeeG left a comment

Choose a reason for hiding this comment

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

This seems like a reasonable fix.

I'd be happy to test this fix if I can think of a way to do that. Or if this gets published in a newer release of atom-package-deps.

@deankevorkian
Copy link
Contributor Author

I simply tested it with atom-ide-base. I built package-deps and copied the Lib to atom-ide-base's node modules folder, disabled and enabled atom-ide-base (retriggering the installation of missing packages).

@DeeDeeG
Copy link

DeeDeeG commented Feb 14, 2021

@deankevorkian @steelbrain I can confirm this fixed the problem for me.

Atom Beta 1.55.0-beta0. macOS 10.15.7.

(A note that is only applicable while live-patching for testing purposes: I actually had to restart Atom to make it load the updated index.js file. But that's just a note for testing, and would not be a problem if this fix is landed and installed via apm.)

@steelbrain steelbrain merged commit b6915f4 into steelbrain:master Feb 15, 2021
@steelbrain
Copy link
Owner

Thank you for working on this!

@steelbrain
Copy link
Owner

Published in atom-package-deps@7.2.1

@deankevorkian deankevorkian deleted the fix-atom-windows-path-spaces branch February 15, 2021 13:25
@DeeDeeG
Copy link

DeeDeeG commented Feb 15, 2021

I can confirm it "just works" when uninstalling and reinstalling atom-ide-base on Atom Beta on macOS.

Thanks to both of you!

@aminya
Copy link
Contributor

aminya commented Feb 16, 2021

This PR has some issues. See this:
atom-community/atom-ide-base#60

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.

Dependencies fail to install when there is a space in Atom's install path
4 participants