Re: [PATCH v2] run-command: add hint when a hook is ignored

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> I think it is easier to reason about if this were not "else if", but
> just a simple "if".

And here are two small suggested changes to the code portion of your
patch.

 - break if / else if cascade into two independent if / if
   statements for clarity.

 - give the "ignored hook" advice only once per <process,hook>
   pair.



 run-command.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/run-command.c b/run-command.c
index 0f8a5f7fa2..0e60dd2075 100644
--- a/run-command.c
+++ b/run-command.c
@@ -5,6 +5,7 @@
 #include "argv-array.h"
 #include "thread-utils.h"
 #include "strbuf.h"
+#include "string-list.h"
 
 void child_process_init(struct child_process *child)
 {
@@ -1170,19 +1171,25 @@ const char *find_hook(const char *name)
 	strbuf_git_path(&path, "hooks/%s", name);
 	if (access(path.buf, X_OK) < 0) {
 		int err = errno;
+
 #ifdef STRIP_EXTENSION
 		strbuf_addstr(&path, STRIP_EXTENSION);
 		if (access(path.buf, X_OK) >= 0)
 			return path.buf;
-		else if (errno == EACCES)
+		if (errno == EACCES)
 			err = errno;
 #endif
 		if (err == EACCES && advice_ignored_hook) {
-			advise(_(
-				"The '%s' hook was ignored because "
-				"it's not set as executable.\n"
-				"You can disable this warning with "
-				"`git config advice.ignoredHook false`."), path.buf);
+			static struct string_list advise_given = STRING_LIST_INIT_DUP;
+
+			if (!string_list_lookup(&advise_given, name)) {
+				string_list_insert(&advise_given, name);
+				advise(_("The '%s' hook was ignored because "
+					 "it's not set as executable.\n"
+					 "You can disable this warning with "
+					 "`git config advice.ignoredHook false`."),
+				       path.buf);
+			}
 		}
 		return NULL;
 	}
-- 
2.15.0-rc0-155-g07e9c1a78d