search

npm / cmd-shim

The cmd-shim used in npm
ISC License
76 stars 40 forks source link

Swap out the slashes for msysgit bash support #4

Closed copenhas closed 9 years ago

copenhas commented 11 years ago

I was unable to use npm to install a node based git command extension. The dirname "$0" command in the shim script always return . due to the path returning being a windows path when running in git bash. After playing around and research I found that other git extensions use sed to swap out the slashes for similar reasons.

Change I used comes from: https://github.com/nvie/gitflow/blob/develop/git-flow#L50

I couldn't come up with a good test to prove the issue and the fix. Everything I tried kept resolving. I did copy the index.js script into my local npm install and reinstalled the troublesome npm package. After that things worked fine. Tested on Windows 7 git bash and normal command prompt.

copenhas commented 10 years ago

Any feedback on this? We have been using the patch at work with success. Allows us to write node.js based git command extensions and install them with npm. Would be nice to not need the patch though. I should mention this is only an issue with Windows.

tiesselune commented 9 years ago

I am having the exact same problem, although my version of cmd-shim is 2.0.1 (and the referenced pull request has been merged since), so I guess this one is still relevant...

The command, installed with npm, works as 

git-something somearg

but fails through git with 

git something somearg

because the path is set to . this is merely annoying for cli extensions, but becomes messy with hooks, filters, and the like, that are invoked directly by git without user input.

Node is a very nice way to write git extensions, so it's a shame npm installation fails. 😩

Is there a reason why this pull request cannot be merged?

copenhas commented 9 years ago

I never received any feedback or message from the maintainer. I also haven't tried the patch with any new releases. We still use this patch manually sometimes at work though.

ForbesLindesay commented 9 years ago

Sorry, the reason I hadn't merged this is because I struggled to figure out whether it works and haven't had much time to really look at it. I'll merge it now under the assumption that you probably know what you're doing. Sorry for the delay.

Would you like to be added as a collaborator on this project, to help review and merge pull requests? This would also help by giving you the ability to fix it if this turns out to break things.

copenhas commented 9 years ago

Thanks for the merge! I don't do a lot of stuff with node.js honestly outside of some tooling at work. You can make me a collaborator but I can give no promises how active I'll be.