Ok, I think this patch should work, though I'll continue testing it 
locally before I merge it. We could also merge this patch sooner if we 
temporarily made the pre-push hook advisory (i.e. doesn't block a push). 
That way, others can try these hooks out without it blocking anyone's 
work. Then when we're happy with it, we can make the pre-push hook error 
out on bad commit messages.

On 4/12/2023 11:49 PM, Eli Zaretskii wrote:
> Please be sure to test the part that finds the file names on all the
> log messages in the repository.  The things people do in logs will
> sometimes surprise you.  In particular, a '*' after leading whitespace
> doesn't necessarily flag a file name, see, for example, the following
> commits:
> 
>    92d75e5c53241ac76e8fdcb6fc66ade68354687c

This works without errors with my latest patch. (Though it's not smart 
enough to recognize the "src/comp.c" as a file to check, since there's 
no leading "* ".) I think that's probably ok; it'd be hard to detect 
cases like that reliably.

>    0bd96806ef1a0f0d2d3f48cdb1204b7e393ab036

This fails, correctly I think. The first line of the commit message is 
"* Rename `comp--typeof-builtin-types'", which I don't think we should 
allow (going forward, at least). Since it's in the first line, we 
*could* treat that specially, but I'm not sure it's worth the 
complexity. Committers can just avoid starting lines with "*" (for 
example, by using " *" instead).

>    eff42dc0af741cc56c52d7d9577d29fc16f9f665

This also works without errors. An indented "*" is ok, and not checked 
by these hooks.

>    b5f70c239e87e5f38fd70181ef75cd28a43a8b41

This fails for two reasons: first, there's a line like this: "* 
buffer-match-p and match-buffers", which is recognized as a list of file 
names. That looks like auto-fill-mode (or something similar) adding a "* 
" where it shouldn't. That's happened to me before, so I'd be glad for 
the hook to catch this.

It also fails because of this line: "* lisp/window.el 
(display-buffer-assq-regexp): Mention what happens". That's correct too, 
since "lisp/window.el" wasn't changed in this commit.

If you apply my patch, you can also test out other commits via "echo 
COMMIT-SHA | awk -f build-aux/git-hooks/commit-msg-files.awk". (I'll do 
this locally as well.)

> Also, it looks like your script doesn't recognize file names in a line
> that starts with a semi-colon, as in this commit:

I fixed this case, though as far as I can tell, authors.el doesn't look 
at lines like this. (I could be wrong, since I just read over that code 
briefly.)

> What about the opposite: a file is mentioned in the diff, but not in
> the commit message?  Or maybe that is allowed, and we shouldn't block
> it?

I think that's ok. Robert Pluim mentions a couple of cases, and we 
probably also want to allow commit messages like: "; Fix last change".