Re: [PATCH v7 05/17] hook: respect hook.runHookDir

[Jonathan Tan <jonathantanmy@google.com>]

> Include hooks specified in the hook directory in the list of hooks to
> run. These hooks do need to be treated differently from config-specified
> ones - they do not need to run in a shell, and later on may be disabled
> or warned about based on a config setting.
> 
> Because they are at least as local as the local config, we'll run them
> last - to keep the hook execution order from global to local.

This commit message doesn't seem to match the code change. Firstly,
we're teaching hook.runHookDir, not respecting it (since it did not
exist before this commit), and it's about showing it in "list" and not
about running it at all.

Perhaps just "hook: teach hook.runHookDir" as the subject and as the
body:

  For now, this just affects the output of "git hook list". In the
  future, this will affect the behavior of "git hook run" and when Git
  runs hooks before or after its operations.

> +	switch (should_run_hookdir) {
> +		case HOOKDIR_NO:
> +			strbuf_addstr(&hookdir_annotation, _(" (will not run)"));
> +			break;
> +		case HOOKDIR_INTERACTIVE:
> +			strbuf_addstr(&hookdir_annotation, _(" (will prompt)"));
> +			break;
> +		case HOOKDIR_WARN:
> +		case HOOKDIR_UNKNOWN:

Hmm...UNKNOWN is the same as WARN? This doesn't agree with what is said
in patch 1:

  +In case this list is expanded in the future, if a value for `hook.runHookDir` is
  +given which Git does not recognize, Git should discard that config entry. For
  +example, if "warn" was specified at system level and "junk" was specified at
  +global level, Git would resolve the value to "warn"; if the only time the config
  +was set was to "junk", Git would use the default value of "yes".

But having said that, I would prefer if Git just errored out in this
case.

The rest looks good.