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

Swap out the slashes for msysgit bash support #4

Merged
merged 1 commit into from
Sep 12, 2015

Conversation

copenhas
Copy link
Contributor

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.

…able path, this is primarily for msysgit bash support
@copenhas
Copy link
Contributor Author

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
Copy link

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
Copy link
Contributor Author

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
Copy link
Contributor

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.

ForbesLindesay added a commit that referenced this pull request Sep 12, 2015
Swap out the slashes for msysgit bash support
@ForbesLindesay ForbesLindesay merged commit 3b99177 into npm:master Sep 12, 2015
@copenhas
Copy link
Contributor Author

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.

@copenhas copenhas deleted the msys.path.fix branch September 14, 2015 19:33
@thefourtheye thefourtheye mentioned this pull request Feb 5, 2016
@kgryte kgryte mentioned this pull request Apr 16, 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.

3 participants