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

Fix whitespace and command-output handling of npm.cmd #6664

Merged
merged 1 commit into from Apr 11, 2016
Merged

Fix whitespace and command-output handling of npm.cmd #6664

merged 1 commit into from Apr 11, 2016

Conversation

dklischies
Copy link
Contributor

The current implementation of the npm wrapper on Windows does not work properly if the username contains a whitespace, because your AppData-path will then contain a whitespace.

Steps to reproduce:

  1. Create a user account with a whitespace in it's name, such that your user directory path will be something like C:\Users\John Doe\
  2. If you run, e.g., meteor npm version you end up with Windows' command not found error.

The same issue exists for the script path/first parameter. I encountered this when using npm-shrinkwrap: You will not trigger Windows' command not found error but module-not-found errors:

   error: couldn't install npm packages from npm-shrinkwrap: Command failed:
   module.js:340
   throw err;
   ^
   Error: Cannot find module 'C:\Users\Daniel'
   at Function.Module._resolveFilename (module.js:338:15)
   at Function.Module._load (module.js:280:25)
   at Function.Module.runMain (module.js:497:10)
   at startup (node.js:119:16)
   at node.js:945:3

I've also added an @-sign. This allows for easier parsing of the output of meteor npm version and the likes via scripts by suppressing CMDs behavior to always write commands in cmd-scripts to the command line. This way it is also consistent with the behavior of those commands on *NIX systems (which is why I would consider the current behavior a bug).

@dklischies
Copy link
Contributor Author

Any feedback on this? Judging from #6679 I'm not the only one experiencing this. To clarify: This bug prevents every windows user from using meteor npm if his or her username contains a blank space. The fix is pretty straightforward. (Not sure who is responsible, maybe @benjamn?)

@benjamn benjamn added this to the Release 1.3.x milestone Apr 11, 2016
@benjamn benjamn self-assigned this Apr 11, 2016
@benjamn benjamn merged commit b7e3ac8 into meteor:devel Apr 11, 2016
benjamn added a commit that referenced this pull request Apr 11, 2016
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

2 participants