-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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 prepare-commit-msg on windows #737
Conversation
Note that the fix (8f05f8a) still works even if you keep the quotes. I removed the quotes in husky.sh (as a separate commit - afac6c4) because it seemed better to have all environments uniformly using the rest param syntax. If the husky.sh change seems to disruptive it can be dropped. For environments where the quoting is working as expected, those arguments will just come through as a single string and the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
squeak
This approach (hand-modified local js to match) is working for me so long as my hooks in "husky": {
"hooks": {
"pre-commit": ".githooks/pre-commit.sh ${HUSKY_GIT_PARAMS[@]}",
"post-commit": ".githooks/post-commit.sh ${HUSKY_GIT_PARAMS[@]}",
"post-checkout": ".githooks/post-checkout.sh ${HUSKY_GIT_PARAMS[@]}",
"post-merge": ".githooks/post-merge.sh ${HUSKY_GIT_PARAMS[@]}"
}
} |
Interesting, I am not finding the array expansion syntax to be necessary. Did you include the |
@whschultz I've added some tests that contradict your results. The shell syntax for array-parameter expansion should not be needed. Let me know if I missed something though. The tests also illustrate that the old behavior was wrong for multi-parameter hooks (viz. Hoping a maintainer will weigh in some week soon... |
@typicode got any feels on a husky release any time soon? |
Just got bit by this. Applied the patch and it worked well. Can a maintainer please have a look at this? |
I can confirm that this change does indeed magically fix issue #735 with Is there any plan to approve and merge this any time soon? @kirkoman is there a workaround for getting |
@rikoe I wasn't aware of that tool until you referenced it. Taking a closer look yeah I would not expect it to function properly on Windows based on #735 and the need for this PR. What I do is to apply a this patch locally when I want to work around this issue. Ultimately it seems related to a platform difference in how shell is parsing arguments so you might find a workaround by playing around with the quotation-escaping on line 17 of husky.sh (but I lost patience looking for a solution with that approach). "$@" husky-run $hookName "$gitParams" |
bump Updated title to reflect that this is a bug fix. Just realized from the tag history that v5 is in progress, and it seems to take a very different approach to managing the hooks. @typicode this is still an ongoing nuisance for myself and presumably the others in the thread. Hoping to get this PR noticed and merged (or at least closed with some feedback). |
Sorry for the late reply. Out of the curiosity, I added your test to master and it seems like the current version is correctly working (https://travis-ci.org/github/typicode/husky/jobs/749056907 search for So I guess, it doesn't need to be merged? PS: thanks for improving the tests |
Cool, thanks for the response. It's a Windows-only bug so the test only fails on Windows. I just ran them locally on the tip of master again to confirm, this is what it looks like for me: ---
hook should run and pass all HUSKY_GIT_PARAMS
---
husky:debug husky v4.3.5 - pre-commit
husky:debug Current working directory is /tmp/husky-project
husky:debug pre-commit config not found, skipping hook
husky:debug husky v4.3.5 - prepare-commit-msg
husky:debug Current working directory is /tmp/husky-project
husky > prepare-commit-msg (node v12.13.1)
prepare-commit-msg hook from Husky
husky:debug npx --no-install husky-run exited with 0 exit code
husky:debug husky v4.3.5 - commit-msg
husky:debug Current working directory is /tmp/husky-project
husky:debug npx --no-install husky-run exited with 0 exit code
husky:debug husky v4.3.5 - post-commit
husky:debug Current working directory is /tmp/husky-project
husky:debug post-commit config not found, skipping hook
[master 1d3c28e] sixth msg
3 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 fifth
create mode 100644 fourth
create mode 100644 sixth
Fail: HUSKY_GIT_PARAMS weren't set correctly
.git/COMMIT_EDITMSG != .git/COMMIT_EDITMSG message
C:\Users\kirkoman\Source\husky> I think it is a difference in how the shell escaping or argument parsing works on Windows. The |
87858e6
to
e9bfce5
Compare
rebased to fix merge conflict and correct my commiter info |
@typicode note that your appveyor windows build did fail that test: https://ci.appveyor.com/project/typicode/husky/builds/36785480 |
I see, thanks for the details. I'll have another look. |
Just gave it a try on Windows, really weird... thanks for catching it and finding a fix! 👍 |
Should fix issue #735