Re: [PATCH v4 1/9] doc: propose hooks managed by the config

For this review, I'll just concern myself with overall design and
structure.

For this patch, overall I think it's better if there's a clear
distinction between what we are implementing now and what we are
implementing later.

+[[motivation]]
+== Motivation
+
+Treat hooks as a first-class citizen by replacing the .git/hook/hookname path as
+the only source of hooks to execute, in a way which is friendly to users with
+multiple repos which have similar needs.

I don't understand what "first-class citizen" here means - probably
better to just omit that phrase and describe the new way of doing hooks.

+[[config-schema-hook]]
+==== `hook`
+
+Primarily contains subsections for each hook event. These order of these
+subsections defines the hook command execution order

The execution order is defined by the order of a multivalue config
variable, I think, not the order of subsections? Besides, I believe that
there's one subsection per hook event (e.g. hook."pre-commit"), not one
subsection per command.

; hook commands can be
+specified by setting the value directly to the command if no additional
+configuration is needed, or by setting the value as the name of a `hookcmd`. If
+Git does not find a `hookcmd` whose subsection matches the value of the given
+command string, Git will try to execute the string directly. Hooks are executed
+by passing the resolved command string to the shell.

[snip]

Hook event subsections can
+also contain per-hook-event settings.

If this is not yet implemented, maybe list under "future work".

+
+Also contains top-level hook execution settings, for example,
+`hook.warnHookDir`, `hook.runHookDir`, or `hook.disableAll`. (These settings are
+described more in <<library,Library>>.)

I think it's clearer if you list this under "future work" - I didn't see
any implementation of this.

+[hook "pre-commit"]
+  command = perl-linter
+  command = /usr/bin/git-secrets --pre-commit
+
+[hook "pre-applypatch"]
+  command = perl-linter
+  error = ignore

Is "error" implemented?

+
+[hook]
+  runHookDir = interactive

Same question for "runHookDir".

+[[config-schema-hookcmd]]
+==== `hookcmd`
+
+Defines a hook command and its attributes, which will be used when a hook event
+occurs. Unqualified attributes are assumed to apply to this hook during all hook
+events, but event-specific attributes can also be supplied. The example runs
+`/usr/bin/lint-it --language=perl <args passed by Git>`, but for repos which
+include this config, the hook command will be skipped for all events to which
+it's normally subscribed _except_ `pre-commit`.
+
+----
+[hookcmd "perl-linter"]
+  command = /usr/bin/lint-it --language=perl
+  skip = true
+  pre-commit-skip = false
+----

And the skips. (And several more below which I will skip.)

+If the caller wants to do something more complicated, the hook library can also
+provide a callback API:
+
+*`int for_each_hookcmd(const char *hookname, hookcmd_function *cb)`*

Is there a use case that would need such a function?

+[[migration]]
+=== Migration path
+
+[[stage-0]]
+==== Stage 0
+
+Hooks are called by running `run-command.h:find_hook()` with the hookname and
+executing the result. The hook library and builtin do not exist. Hooks only
+exist as specially named scripts within `.git/hooks/`.
+
+[[stage-1]]
+==== Stage 1
+
+`git hook list --porcelain <hook-event>` is implemented. Users can replace their
+`.git/hooks/<hook-event>` scripts with a trampoline based on `git hook list`'s
+output. Modifier commands like `git hook add` and `git hook edit` can be
+implemented around this time as well.

This seems to contradict patch 8, which teaches Git to use the configs
directly without any change to .git/hooks/<hook-event> (at least for
certain commit-related hooks).

+[[future-work]]
+== Future work
+
+[[execution-ordering]]
+=== Execution ordering
+
+We may find that config order is insufficient for some users; for example,
+config order makes it difficult to add a new hook to the system or global config
+which runs at the end of the hook list. A new ordering schema should be:
+
+1) Specified by a `hook.order` config, so that users will not unexpectedly see
+their order change;
+
+2) Either dependency or numerically based.
+
+Dependency-based ordering is prone to classic linked-list problems, like a
+cycles and handling of missing dependencies. But, it paves the way for enabling
+parallelization if some tasks truly depend on others.
+
+Numerical ordering makes it tricky for Git to generate suggested ordering
+numbers for each command, but is easy to determine a definitive order.

With this schema, and with the "skip" behavior described above (but not
implemented in this patch set), rudimentary ordering can already be
done; because a hook is removed and reinserted whenever it appears in
the config, even a hook X in the system config can be made to run after a
hook Y in the worktree config by adding Y then X in the worktree config,
and if we want to disable X instead, we can just add "skip" to X.