Hi Emily,
On Wed, 9 Sep 2020 at 02:54, Emily Shaffer <emilyshaffer@xxxxxxxxxx> wrote:
> DESCRIPTION
> -----------
> You can list, add, and modify hooks with this command.
(BTW, I think this patch could teach this to say "You can list hooks
with this command." If/when we add the other commands, we can expand
on this.)
> +This command parses the default configuration files for sections "hook" and
> +"hookcmd". "hook" is used to describe the commands which will be run during a
I propose s/"hook"/`hook`/ and similar to set this as monospace since we
are discussing configuration sections. If we want to avoid starting
sentences with "hook" (or `hookcmd`; do we?), maybe something like "The
section `hook` ..." would work fine.
> +particular hook event; commands are run in config order. "hookcmd" is used to
"config order" feels a bit too colloquial/vague. You use the same phrase
in the commit message and I think it works well there for the indented
audience. But for this document, I'm not so sure. How about
Commands are run in the order they are encountered as the Git
configuration files are processed (see linkgit:git-config[1]).
? It's also quite possible that "config order" hits the exact right tone
-- please trust your judgment.
> +describe attributes of a specific command. If additional attributes don't need
> +to be specified, a command to run can be specified directly in the "hook"
> +section; if a "hookcmd" by that name isn't found, Git will attempt to run the
> +provided value directly. For example:
> + [hook "post-commit"]
> + command = "linter"
> + command = "~/typocheck.sh"
> +
> + [hookcmd "linter"]
> + command = "/bin/linter --c"
Hmm. "hook", "command" and "hookcmd". Should that be "cmd", or
"hookcommand"? I'd favour the latter, but the current proposal somehow
feels asymmetric. (If code uses, and is consistent about using,
"hookcmd" that's another thing entirely, I think. It's just that for the
configuration, it looks a bit odd.)
> +List the hooks which have been configured for <hook-name>. Hooks appear
`<hook-name>` with backticks.
> +in the order they should be run, and note the config scope where the relevant
> +`hook.<hook-name>.command` was specified, not the `hookcmd` (if applicable).
I had to read and re-read this a few times. The "and note the" does not
mean "and please observe that", but rather "and they make note of". Not
sure how that can be done clearer. The second thing that tripped me up
was that last part. Maybe end the sentence after "specified", then add
something like "The scope is not affected by if and where
`hookcmd.<hook-name>.command` appears.".
I think you could add
CONFIGURATION
-------------
include::config/hook.txt[]
here and add such a file
hook.<hook-name>.command::
...
hookcmd.<hook-name>.command::
...
where you define/describe those items. And you can include it from
config.txt as well.
Martin
