observing/pre-commit

`env node` doesn't work well with GUI git clients

Closed this issue · 17 comments

node isn't always in the PATH for GUI clients.

Got a suggestion to fix it?

Either we assume that node is always in /usr/local/bin/node, or we do magic :)

Example of magic:

node_runner:

#!/bin/bash

# check for node in PATH, if not assume `/usr/local/bin/node`,
# read from config, etc

# do node $1

pre-commit

#!/usr/bin/env ./node_runner

// javascript goes here

I've considered that but the problem is it would be a platform lock-in for unix as windows does not have bash.

How about #!/usr/bin/env on windows? :) And git-hooks in general. :) (It's still unsolved problem in our company, people just ssh to linux box).

Or if bash is the problem, we can do politically correct #!/usr/bin/env sh :)

I could just set the correct during install... Like do a find replace of the shebang with location of node process that executes the current installation if cannot the default location of it. That would probably be best of both worlds

On 11 Aug 2014, at 23:16, Alex Indigo notifications@github.com wrote:

How about #!/usr/bin/env on windows? :) And git-hooks in general. :) (It still unsolved problem in our company, people just ssh to linux box).

Or if bash is the problem, we can do politically correct #!/usr/bin/env sh :)


Reply to this email directly or view it on GitHub.

Great idea :) Thanks.

My concerns about windows were invalid as well, windows doesn't even support shebangs unless it's running Cygwin.

Could you check if the discovery branch works as intended for you?

This is what I meant in my (somewhat) snarky comment. :)
Thank you, I'll play with it today. 👍

@alexindigo Sorry about that, my brain sometimes ignores snarky comments so I totally missed that remark ;-)

And I'm still learning to talk to humans properly. :)

Hey, it doesn't work as we thought it would. I do npm install from command line, where I do have node in the PATH. :)

You solution wouldn't work either then I guess.

Something like that – #13
Works in both CLI and GUI.

👍

Could you confirm if master is working as expected?

Please re-open if I still messed things up.