-
Notifications
You must be signed in to change notification settings - Fork 16
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
respect core.hooksPath, falling back to .git/hooks #12
base: master
Are you sure you want to change the base?
Conversation
@@ -624,7 +624,10 @@ proc git_write {args} { | |||
} | |||
|
|||
proc githook_read {hook_name args} { | |||
set pchook [gitdir hooks $hook_name] | |||
if {[catch {set hooksdir [git config core.hooksPath]}]} { |
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.
There is a get_config
function that uses a cached config (you need to use lower-case keys for that, though, IIRC).
In any case, this is duplicating the logic in Git's config.c
, therefore a bit fragile, besides, if Git GUI is used with Git <v2.9.0 it would be inconsistent with Git's idea of the hooks path.
How about something like this instead (completely untested, you likely have to fix a few bits here and there):
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 37c1c5d227b..3067a3b000a 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -624,7 +624,11 @@ proc git_write {args} {
}
proc githook_read {hook_name args} {
- set pchook [gitdir hooks $hook_name]
+ if {[package vcompare $::_git_version 2.5.0] >= 0} {
+ set pchook [git rev-parse --git-path "hooks/$hook_name"]
+ } else {
+ set pchook [gitdir hooks $hook_name]
+ }
lappend args 2>@1
# On Windows [file executable] might lie so we need to ask
I offered the same patch on the Git mailing list, together with a suggestion for an improved commit message.
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.
How about making it so by amending your patch and force-pushing, just in case that Pat returns from his absence and wants to merge the open PRs?
Hi @mephinet. I offered an alternative, also on the Git mailing list. Since this repository has become very, very quiet, I would understand if you went back to trying to get this patch in via the Git mailing list, side-stepping @patthoyts. If you want to, I would be willing to carry the patch (after the discussion settles) in Git for Windows (not sure what OS you use, so I don't know whether that would help you). |
@mephinet hi? |
This fixes git-for-windows/git#1755. |
@luismbo could I bother you to test git-for-windows/git#1757, as this here PR has been stalled from two sides already? |
@dscho you're very welcome to forward the patch. |
The following patch tries to fix git-gui to respect the core.hooksPath config
variable, falling back to the old behavior.