search

benjaminRomano / grunt-manager

Manage gruntfiles from inside Atom
https://atom.io/packages/grunt-manager
MIT License
0 stars 1 forks source link

Fix loading of local grunt on windows #7

Open meetjey opened 8 years ago

meetjey commented 8 years ago

Edit test to rewrite process.env.PATH only on platform which aren't win32.

benjaminRomano commented 8 years ago

Is this a fix for #7? From just looking at the code, it seems like it's doing the same thing as before just in a cleaner way.

meetjey commented 8 years ago

It's a fix for #6 on windows PATH is not set by default, so actually it force PATH to null (there is also "Path" on Windows which is set) So i remove the test for Windows in that fix, and PATH stay unset and it works for local or global Grunt. I don't know if it's really clear ^^

benjaminRomano commented 8 years ago

I'll take a look at this over the weekend.

benjaminRomano commented 8 years ago

Sorry for taking so long to take a look at this. I pulled down the code locally, but it didn't work for me.

I believe the correct way to fix this is to have the command here: https://github.com/benjaminRomano/grunt-manager/blob/master/lib/gruntfile-runner.coffee#L55 be set to 

/<path-to-project-dir>/node_modules/.bin/grunt

However, I believe that supporting locally installed grunt executables is a bad idea.

Suppose you have two projects open, Project1 and Project2. Assume Project1 has grunt installed locally, but Project1 does not. Would Project2 be able to use grunt by borrowing Project1's grunt executable?

Personally, I'd say that this behavior shouldn't be allowed, but that requires a lot of code to do checking and handling for that.