From e26217843aca5867bc550752ba2628d91b02a5c0 Mon Sep 17 00:00:00 2001 From: "Gustavo L. de M. Chaves" Date: Sat, 13 May 2023 11:17:11 -0300 Subject: [PATCH 1/9] Use warn/die instead of carp/croak When reporting checking errors to the user, the fail_on_faults routine was using the carp/croak methods, which always shows the filename and line number where the error occurs, even when the error message ends in a newline. We don't need this information because it always shows the same place in which the routine is invoked. Thanks @nugged for bringing this to my attention. [0] Ref0: https://github.com/gnustavo/Git-Hooks/commit/6795420aa3dccca66f55eee69ada3793d4bd001d#r113136608 --- lib/Git/Repository/Plugin/GitHooks.pm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Git/Repository/Plugin/GitHooks.pm b/lib/Git/Repository/Plugin/GitHooks.pm index 34d82f2..f67f763 100644 --- a/lib/Git/Repository/Plugin/GitHooks.pm +++ b/lib/Git/Repository/Plugin/GitHooks.pm @@ -891,10 +891,10 @@ sub fail_on_faults { $faults .= "\n" unless $faults =~ /\n$/; if ($warn_only) { $log->warning(Warning => {faults => $faults}); - carp $faults; + warn $faults; } else { $log->error(Error => {faults => $faults}); - croak $faults; + die $faults; } } From 78bc849dfb576ad7ae1b437974be337bb39150b9 Mon Sep 17 00:00:00 2001 From: Bartosz Oudekerk Date: Tue, 19 Sep 2023 12:55:23 +0200 Subject: [PATCH 2/9] Allow plugins to be OR-ed. --- lib/Git/Repository/Plugin/GitHooks.pm | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/Git/Repository/Plugin/GitHooks.pm b/lib/Git/Repository/Plugin/GitHooks.pm index f67f763..9ec842a 100644 --- a/lib/Git/Repository/Plugin/GitHooks.pm +++ b/lib/Git/Repository/Plugin/GitHooks.pm @@ -384,7 +384,7 @@ sub load_plugins { my %plugins; - foreach my $plugin (map {split} $git->get_config(githooks => 'plugin')) { + foreach my $plugin (map {split /[\|\s]/} $git->get_config(githooks => 'plugin')) { my ($negation, $prefix, $basename) = ($plugin =~ /^(\!?)((?:.+::)?)(.+)/); if (exists $ENV{$basename} && ! $ENV{$basename}) { @@ -853,7 +853,18 @@ sub get_faults { $faults .= $colors->{header} . qx{$header} . "$colors->{reset}\n"; ## no critic (ProhibitBacktickOperators) } - $faults .= join("\n\n", @{$git->{_plugin_githooks}{faults}}); + my @condition = split(" ", $git->get_config(githooks => 'plugin')); + + foreach (@{$git->{_plugin_githooks}{faults}}) { + my $plugin = ''; + ($plugin) = $_ =~ /.*Git::Hooks::(\w+):.*/; + map {s/$plugin//} @condition; + } + + # If any parts of the condition are empty they have failed and we should return all faults + if (grep {/^(\|*)$/} @condition) { + $faults .= join("\n\n", @{$git->{_plugin_githooks}{faults}}); + } if ($git->{_plugin_githooks}{hookname} =~ /^commit-msg|pre-commit$/ && ! $git->get_config_boolean(githooks => 'abort-commit')) { From a8bcd33200e0e61b1ad0e3b6d10f93f1c5a69cb2 Mon Sep 17 00:00:00 2001 From: Bartosz Oudekerk Date: Tue, 19 Sep 2023 12:55:54 +0200 Subject: [PATCH 3/9] Update documentation --- lib/Git/Hooks.pm | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/Git/Hooks.pm b/lib/Git/Hooks.pm index 909df8b..d575b5b 100644 --- a/lib/Git/Hooks.pm +++ b/lib/Git/Hooks.pm @@ -1278,6 +1278,13 @@ is useful if you want to enable a plugin globally and only disable it for some repositories. You can even re-enable it later, if you want, by mentioning it again without the prefix. +Plugins can be OR-ed by separating them with a pipe (C<|>) instead of a space. + + [githooks] + plugin = CheckFile|CheckJira + +In this case either CheckFile C CheckJira has to match. + =head2 [DEPRECATED after v3.3.0] disable PLUGIN... B Please, use the C option with an From af39d5cfff4e5eb053aec49a528b021c25422535 Mon Sep 17 00:00:00 2001 From: Bartosz Oudekerk Date: Tue, 19 Sep 2023 13:07:52 +0200 Subject: [PATCH 4/9] Example now shows it can be used in addition to the old behaviour --- lib/Git/Hooks.pm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Git/Hooks.pm b/lib/Git/Hooks.pm index d575b5b..62acf1a 100644 --- a/lib/Git/Hooks.pm +++ b/lib/Git/Hooks.pm @@ -1281,9 +1281,9 @@ again without the prefix. Plugins can be OR-ed by separating them with a pipe (C<|>) instead of a space. [githooks] - plugin = CheckFile|CheckJira + plugin = CheckFile|CheckJira CheckWhitespace -In this case either CheckFile C CheckJira has to match. +In this case CheckWhitespace C either CheckFile C CheckJira has to match. =head2 [DEPRECATED after v3.3.0] disable PLUGIN... From 1adfe39f5bb5a9f14a38fdc8b149b57e16a39ea9 Mon Sep 17 00:00:00 2001 From: Bartosz Oudekerk Date: Tue, 19 Sep 2023 16:28:25 +0200 Subject: [PATCH 5/9] plugin is used elsewhere as variable, renaming for clarity --- lib/Git/Repository/Plugin/GitHooks.pm | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Git/Repository/Plugin/GitHooks.pm b/lib/Git/Repository/Plugin/GitHooks.pm index 9ec842a..87ae4e9 100644 --- a/lib/Git/Repository/Plugin/GitHooks.pm +++ b/lib/Git/Repository/Plugin/GitHooks.pm @@ -856,9 +856,9 @@ sub get_faults { my @condition = split(" ", $git->get_config(githooks => 'plugin')); foreach (@{$git->{_plugin_githooks}{faults}}) { - my $plugin = ''; - ($plugin) = $_ =~ /.*Git::Hooks::(\w+):.*/; - map {s/$plugin//} @condition; + my $origin = ''; + ($origin) = $_ =~ /.*Git::Hooks::(\w+):.*/; + map {s/$origin//} @condition; } # If any parts of the condition are empty they have failed and we should return all faults From 6e80c3c87eeafac4315e03544a89c340cf963f19 Mon Sep 17 00:00:00 2001 From: Bartosz Oudekerk Date: Tue, 19 Sep 2023 16:29:04 +0200 Subject: [PATCH 6/9] No need for the grouping --- lib/Git/Repository/Plugin/GitHooks.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Git/Repository/Plugin/GitHooks.pm b/lib/Git/Repository/Plugin/GitHooks.pm index 87ae4e9..d248c93 100644 --- a/lib/Git/Repository/Plugin/GitHooks.pm +++ b/lib/Git/Repository/Plugin/GitHooks.pm @@ -862,7 +862,7 @@ sub get_faults { } # If any parts of the condition are empty they have failed and we should return all faults - if (grep {/^(\|*)$/} @condition) { + if (grep {/^\|*$/} @condition) { $faults .= join("\n\n", @{$git->{_plugin_githooks}{faults}}); } From 4d0586ee3e76f9c7841bd3a7bcb1ec1501c32115 Mon Sep 17 00:00:00 2001 From: Bartosz Oudekerk Date: Wed, 20 Sep 2023 12:53:16 +0200 Subject: [PATCH 7/9] Log how the different conditions were evaluated --- lib/Git/Repository/Plugin/GitHooks.pm | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/Git/Repository/Plugin/GitHooks.pm b/lib/Git/Repository/Plugin/GitHooks.pm index d248c93..87ae8af 100644 --- a/lib/Git/Repository/Plugin/GitHooks.pm +++ b/lib/Git/Repository/Plugin/GitHooks.pm @@ -853,17 +853,19 @@ sub get_faults { $faults .= $colors->{header} . qx{$header} . "$colors->{reset}\n"; ## no critic (ProhibitBacktickOperators) } - my @condition = split(" ", $git->get_config(githooks => 'plugin')); + my @conditions = split(" ", $git->get_config(githooks => 'plugin')); + map {s/(\||$)/(ok)$1/g} @conditions; foreach (@{$git->{_plugin_githooks}{faults}}) { my $origin = ''; ($origin) = $_ =~ /.*Git::Hooks::(\w+):.*/; - map {s/$origin//} @condition; + map {s/($origin)\(ok\)/$1(failed)/g} @conditions; } - # If any parts of the condition are empty they have failed and we should return all faults - if (grep {/^\|*$/} @condition) { + # If any parts of the condition still have an 'OK', they succeeded, otherwise we fail and need to log the faults + if (grep {!/\(ok\)/} @conditions) { $faults .= join("\n\n", @{$git->{_plugin_githooks}{faults}}); + $faults .= join(" ", "\n\nOur hooks as they were evaluated:\n", @conditions, "\n\n"); } if ($git->{_plugin_githooks}{hookname} =~ /^commit-msg|pre-commit$/ From f49bc97d5d19b83de1ef6d87b1f7ab77f6df9ba9 Mon Sep 17 00:00:00 2001 From: Bartosz Oudekerk Date: Wed, 27 Sep 2023 16:03:48 +0200 Subject: [PATCH 8/9] Pass along the name of the plugin --- lib/Git/Repository/Plugin/GitHooks.pm | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/Git/Repository/Plugin/GitHooks.pm b/lib/Git/Repository/Plugin/GitHooks.pm index 87ae8af..b0f1c8a 100644 --- a/lib/Git/Repository/Plugin/GitHooks.pm +++ b/lib/Git/Repository/Plugin/GitHooks.pm @@ -803,10 +803,11 @@ sub fault { my $colors = _githooks_colors($git); my $msg; + my $prefix; { - my $prefix = $info->{prefix} || caller; my @context; + $prefix = $info->{prefix} || caller; if (my $commit = $info->{commit}) { $commit = $commit->commit if ref $commit; # It's a Git::Repository::Log object @@ -834,7 +835,7 @@ sub fault { $msg .= "\n$colors->{details}$details$colors->{reset}\n\n"; } - push @{$git->{_plugin_githooks}{faults}}, $msg; + push @{$git->{_plugin_githooks}{faults}}, {msg => $msg, plugin => ($prefix) =~ /.*Git::Hooks::(\w+)/}; # Return true to allow for the idiom: or $git->fault(...) and ; return 1; @@ -856,16 +857,14 @@ sub get_faults { my @conditions = split(" ", $git->get_config(githooks => 'plugin')); map {s/(\||$)/(ok)$1/g} @conditions; - foreach (@{$git->{_plugin_githooks}{faults}}) { - my $origin = ''; - ($origin) = $_ =~ /.*Git::Hooks::(\w+):.*/; - map {s/($origin)\(ok\)/$1(failed)/g} @conditions; + foreach my $fault (@{$git->{_plugin_githooks}{faults}}) { + map {s/($fault->{plugin})\(ok\)/$1(failed)/g} @conditions; } - # If any parts of the condition still have an 'OK', they succeeded, otherwise we fail and need to log the faults + # If any parts of the condition don't have an 'OK', they've failed and we need to deny the commit if (grep {!/\(ok\)/} @conditions) { - $faults .= join("\n\n", @{$git->{_plugin_githooks}{faults}}); - $faults .= join(" ", "\n\nOur hooks as they were evaluated:\n", @conditions, "\n\n"); + $faults .= join("\n\n", map {$_->{msg}} @{$git->{_plugin_githooks}{faults}}); + $faults .= join(" ", "\n\nOur configured plugins as they were evaluated:\n", @conditions, "\n\n"); } if ($git->{_plugin_githooks}{hookname} =~ /^commit-msg|pre-commit$/ From 4d734683805941f0335e4187cac72bc9309c2dda Mon Sep 17 00:00:00 2001 From: Bartosz Oudekerk Date: Fri, 29 Sep 2023 11:12:58 +0200 Subject: [PATCH 9/9] Only join array to avoid unnecessary whitespace --- lib/Git/Repository/Plugin/GitHooks.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Git/Repository/Plugin/GitHooks.pm b/lib/Git/Repository/Plugin/GitHooks.pm index b0f1c8a..69e9e06 100644 --- a/lib/Git/Repository/Plugin/GitHooks.pm +++ b/lib/Git/Repository/Plugin/GitHooks.pm @@ -864,7 +864,7 @@ sub get_faults { # If any parts of the condition don't have an 'OK', they've failed and we need to deny the commit if (grep {!/\(ok\)/} @conditions) { $faults .= join("\n\n", map {$_->{msg}} @{$git->{_plugin_githooks}{faults}}); - $faults .= join(" ", "\n\nOur configured plugins as they were evaluated:\n", @conditions, "\n\n"); + $faults .= "\n\nOur configured plugins as they were evaluated:\n" . join(" ", @conditions) . "\n\n"; } if ($git->{_plugin_githooks}{hookname} =~ /^commit-msg|pre-commit$/