bug#68691: 30.0.50; [WISHLIST] Make it easier to conform to desired commit message format

bug#68691: 30.0.50; [WISHLIST] Make it easier to conform to desired commit message format

[No Wayman]

Idea: Extend checkdoc or write a new minor mode that runs 
stylistic checks on commit messages. 

bug#68691: 30.0.50; [WISHLIST] Make it easier to conform to desired commit message format

[Stefan Kangas]

No Wayman <iarchivedmywholelife@gmail.com> writes:

> Idea: Extend checkdoc or write a new minor mode that runs
> stylistic checks on commit messages.

Since we try to adhere to the GNU ChangeLog format, I think such a
linting mode would help contributors.

Copying in Po Lu, who might be interested in working on it.

FWIW, I'll not be working on such a mode myself, as I'd rather see that
we promoted useful commit messages in the style of, say, the Linux
kernel instead of the GNU ChangeLog format.  It is simply redundant to
enumerate changed files when using a modern distributed VCS like Git.
But that's my personal opinion, and not currently that of the project.

bug#68691: 30.0.50; [WISHLIST] Make it easier to conform to desired commit message format

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Kangas <stefankangas@gmail.com> writes:

> FWIW, I'll not be working on such a mode myself, as I'd rather see that
> we promoted useful commit messages in the style of, say, the Linux
> kernel instead of the GNU ChangeLog format.  It is simply redundant to
> enumerate changed files when using a modern distributed VCS like Git.
> But that's my personal opinion, and not currently that of the project.

I think part of our position is that it is far too easy to write
uninformative commit messages when one is not being forced to mentally
review the commit's contents.  As with any other form of discipline,
maintaining ChangeLog discipline is an end in itself, not necessarily
something that might produce an immediate benefit.  See FreeType's
experience:

  https://lists.nongnu.org/archive/html/freetype-devel/2013-04/msg00046.html
  https://lists.nongnu.org/archive/html/freetype-devel/2020-07/msg00032.html

A hook run before each commit is a good idea, and I plan to study how
they are written.

bug#68691: 30.0.50; [WISHLIST] Make it easier to conform to desired commit message format

[Eli Zaretskii]

> Cc: Po Lu <luangruo@yahoo.com>
> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Thu, 25 Jan 2024 15:09:22 -0800
> 
> FWIW, I'll not be working on such a mode myself, as I'd rather see that
> we promoted useful commit messages in the style of, say, the Linux
> kernel instead of the GNU ChangeLog format.  It is simply redundant to
> enumerate changed files when using a modern distributed VCS like Git.
> But that's my personal opinion, and not currently that of the project.

The reason we keep the ChangeLog format is that then the ChangeLog
files generated from the Git logs and included in the release tarballs
are useful on their own, without the need to use Git and have the
repository cloned on the end user's machine.  Admittedly, systems
where Emacs is installed but Git access to our repository is limited
or non-existent are relatively rare these days, but they do exist (I
personally have to work on such a system, FWIW).  Being able to grep
the ChangeLog files locally is an advantage in those cases.

We have in Emacs several commands that help producing the log messages
according to our conventions, so the only reasons for people not to
provide such commit logs that I think of are:

  . our conventions are unknown to the contributor
  . the contributor is not familiar with Emacs commands that help with
    producing well-formatted commit log messages
  . the contributor doesn't use Emacs to edit the code

bug#68691: 30.0.50; [WISHLIST] Make it easier to conform to desired commit message format

[Jim Porter]

On 1/25/2024 5:27 PM, Po Lu via Bug reports for GNU Emacs, the Swiss 
army knife of text editors wrote:
> A hook run before each commit is a good idea, and I plan to study how
> they are written.

We already have a few hooks for this, including a `commit-msg` hook that 
checks for various formatting issues, and `post-commit` + `pre-push` 
hooks to verify that the files listed in the message match the diff. 
(There are some annoying technical reasons for using `post-commit` and 
`pre-push` for this, which are documented in their implementations under 
build-aux/git-hooks/.)

You could probably consult the existing implementations to see how to do 
things, and improve them with any extra features that might help here.

bug#68691: 30.0.50; [WISHLIST] Make it easier to conform to desired commit message format

[Stefan Kangas]

Po Lu <luangruo@yahoo.com> writes:

> I think part of our position is that it is far too easy to write
> uninformative commit messages when one is not being forced to mentally
> review the commit's contents.  As with any other form of discipline,
> maintaining ChangeLog discipline is an end in itself, not necessarily
> something that might produce an immediate benefit.

I'm not buying it, sorry.  Useless commit messages are common in all GNU
projects, including Emacs.

It does not make it better that the GNU ChangeLog format is so noisy
that it's hard to pick out the good ones from the bad ones.  It makes it
easy to disguise a very bad commit message as a good one.

> A hook run before each commit is a good idea, and I plan to study how
> they are written.

We do not want a git hook for this, since we don't ask that all commits
follow this format.  It would be too heavy-handed, not to mention
annoying when doing development locally.

The request is for an Emacs minor mode or command that can help users
follow the format.

bug#68691: 30.0.50; [WISHLIST] Make it easier to conform to desired commit message format

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> The reason we keep the ChangeLog format is that then the ChangeLog
> files generated from the Git logs and included in the release tarballs
> are useful on their own, without the need to use Git and have the
> repository cloned on the end user's machine.  Admittedly, systems
> where Emacs is installed but Git access to our repository is limited
> or non-existent are relatively rare these days, but they do exist (I
> personally have to work on such a system, FWIW).  Being able to grep
> the ChangeLog files locally is an advantage in those cases.

I'm not saying users that read them don't exist, I just think they're
rare.  My guess is that ChangeLog files are consulted far less often
than NEWS, and we know how often that happens.

But if we still consider these files useful, I think we could also
create them starting from something like `git log --stat', and then
generating the rest.  We wouldn't get a perfect result, perhaps, but we
could probably get 99 % of the way there if we really wanted to.

IOW, I think the question of distributing ChangeLog files could be
considered separately from their exact format.

bug#68691: 30.0.50; [WISHLIST] Make it easier to conform to desired commit message format

[Eli Zaretskii]

> Cc: No Wayman <iarchivedmywholelife@gmail.com>, 68691@debbugs.gnu.org
> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Sat, 27 Jan 2024 16:44:24 -0800
> 
> > A hook run before each commit is a good idea, and I plan to study how
> > they are written.
> 
> We do not want a git hook for this, since we don't ask that all commits
> follow this format.  It would be too heavy-handed, not to mention
> annoying when doing development locally.

I actually don't believe this can be done well enough to be useful,
i.e. with high-enough true positive rate and low enough false positive
rate.  Whether a commit log message is good or bad is basically a
human judgment call; software-based solutions for that, if at all
possible, can only be based on machine-learning techniques, which
INSHO would be a ridiculously expensive solution for such a simple
problem.

bug#68691: 30.0.50; [WISHLIST] Make it easier to conform to desired commit message format

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Sat, 27 Jan 2024 17:07:25 -0800
> Cc: iarchivedmywholelife@gmail.com, 68691@debbugs.gnu.org, luangruo@yahoo.com
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > The reason we keep the ChangeLog format is that then the ChangeLog
> > files generated from the Git logs and included in the release tarballs
> > are useful on their own, without the need to use Git and have the
> > repository cloned on the end user's machine.  Admittedly, systems
> > where Emacs is installed but Git access to our repository is limited
> > or non-existent are relatively rare these days, but they do exist (I
> > personally have to work on such a system, FWIW).  Being able to grep
> > the ChangeLog files locally is an advantage in those cases.
> 
> I'm not saying users that read them don't exist, I just think they're
> rare.  My guess is that ChangeLog files are consulted far less often
> than NEWS, and we know how often that happens.
> 
> But if we still consider these files useful, I think we could also
> create them starting from something like `git log --stat', and then
> generating the rest.  We wouldn't get a perfect result, perhaps, but we
> could probably get 99 % of the way there if we really wanted to.

The script which generates ChangeLog files from Git logs is not
maintained by us, so this suggestion should go to the Gnulib folks, I
think.  If they succeed in generating the files and variables touched
by a given change mechanically, we can modify our conventions to ask
contributors to provide only the part that explains the change, its
rationale and important aspects, something that a program cannot
produce.

bug#68691: 30.0.50; [WISHLIST] Make it easier to conform to desired commit message format

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> I actually don't believe this can be done well enough to be useful,
> i.e. with high-enough true positive rate and low enough false positive
> rate.

I tend to agree, though what I had in mind was something simpler than
that: to opportunistically catch some obvious errors.

For example, we could highlight text going over 72/80 columns, or mark
in red any line matching "^lisp/foo/bar.el" (which should be a mistake
99.9 % of the time).  Stuff like that.

I'm not sure that it will be hugely useful in the end, but it might be
worth a try to hack something up.

bug#68691: 30.0.50; [WISHLIST] Make it easier to conform to desired commit message format

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> I'm not saying users that read them don't exist, I just think they're
>> rare.  My guess is that ChangeLog files are consulted far less often
>> than NEWS, and we know how often that happens.

Presumably very often?

>> But if we still consider these files useful, I think we could also
>> create them starting from something like `git log --stat', and then
>> generating the rest.  We wouldn't get a perfect result, perhaps, but we
>> could probably get 99 % of the way there if we really wanted to.
>
> The script which generates ChangeLog files from Git logs is not
> maintained by us, so this suggestion should go to the Gnulib folks, I
> think.  If they succeed in generating the files and variables touched
> by a given change mechanically, we can modify our conventions to ask
> contributors to provide only the part that explains the change, its
> rationale and important aspects, something that a program cannot
> produce.

How is this different from expecting them to type C-c C-w in a vc-log
buffer, and inserting this information?

BTW, I wasn't proposing a commit hook that verifies the _contents_ of
each commit message, only one that verifies the format of the messages.

bug#68691: 30.0.50; [WISHLIST] Make it easier to conform to desired commit message format

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Sat, 27 Jan 2024 23:21:47 -0800
> Cc: luangruo@yahoo.com, iarchivedmywholelife@gmail.com, 68691@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I actually don't believe this can be done well enough to be useful,
> > i.e. with high-enough true positive rate and low enough false positive
> > rate.
> 
> I tend to agree, though what I had in mind was something simpler than
> that: to opportunistically catch some obvious errors.
> 
> For example, we could highlight text going over 72/80 columns

This is already done in one of the commit hooks.

> or mark in red any line matching "^lisp/foo/bar.el" (which should be
> a mistake 99.9 % of the time).

If that names a nonexistent file, or a file with wrong leading
directories, one of the existing hooks already does catch such
mistakes.

bug#68691: 30.0.50; [WISHLIST] Make it easier to conform to desired commit message format

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Stefan Kangas <stefankangas@gmail.com>,  iarchivedmywholelife@gmail.com,
>   68691@debbugs.gnu.org
> Date: Sun, 28 Jan 2024 15:26:18 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > The script which generates ChangeLog files from Git logs is not
> > maintained by us, so this suggestion should go to the Gnulib folks, I
> > think.  If they succeed in generating the files and variables touched
> > by a given change mechanically, we can modify our conventions to ask
> > contributors to provide only the part that explains the change, its
> > rationale and important aspects, something that a program cannot
> > produce.
> 
> How is this different from expecting them to type C-c C-w in a vc-log
> buffer, and inserting this information?

It's one thing less to do.

> BTW, I wasn't proposing a commit hook that verifies the _contents_ of
> each commit message, only one that verifies the format of the messages.

It cannot.  Format could be okay, but will not describe all of the
changes, or describe them in a nonsensical way.

OTOH, is the format below OK or not?

     Merge from origin/emacs-29

     53481cc9546 Fix description of when "\xNNN" is considered a unibyte character
     1ef8b90ae06 Simplify imenu setup for {cmake,dockerfile}-ts-modes
     7338af9c986 ; * etc/PROBLEMS: Document that GnuPG 2.4.4 solves the EasyPG
     5483a1df99c Improve documentation of profiler commands
     fb4cf0ab46d ; Fix xref under Output Overrides in Elisp manual.
     aa6c24da61f Fix broken links to Freedesktop notifications spec
     14d68221d26 Fix nasty cut'n'waste error in Tramp
     51ca049608c Fix image-dired-tags-db-file void variable error
     c450eec07ff typescript-ts-mode: Skip test if tsx grammar missing
     9841ced147f ; Fix typos
     557ed9c0463 * admin/README: Document the run-codespell script.
     5701f96335c * admin/README: Fix entry on coccinelle subdirectory.
     1805f4bfd62 Add script admin/run-codespell and supporting files

Or this one:

    Revert "Fix typo in lispref 'Creating Strings' section"

    This reverts commit b825962ea840348bbde0c834ca398458a06fbb8b
    which was mistakenly installed in master instead of emacs-29.

Or this one:

    ; Update copyright years in more files

How do you distinguish between the above perfectly valid examples, and
some arbitrary text in a log message (which should be flagged as
nonconforming)?

bug#68691: 30.0.50; [WISHLIST] Make it easier to conform to desired commit message format

[Juri Linkov]

> or mark in red any line matching "^lisp/foo/bar.el" (which should be
> a mistake 99.9 % of the time).  Stuff like that.

There is already some linting in log-edit that could be extended further:

  (defconst log-edit-font-lock-gnu-keywords
      ;; Use
      ;;   * foo.el (bla, bli)
      ;;   (blo, blu): Toto.
      ;; Rather than
      ;;   * foo.el (bla, bli,
      ;;   blo, blu): Toto.
    '(("^[ \t]*\\(?:\\* .*\\)?\\(([^\n)]*,\\s-*\\)$"
       (1 '(face font-lock-warning-face
            help-echo "Continue function lists with \")\\n(\".") t))
      ;; Don't leave a lone word on a single line.
      ;;("^\\s-*\\(\\S-*[^\n:)]\\)\\s-*$" (1 font-lock-warning-face t))
      ;; Don't cut a sentence right after the first word (better to move
      ;; the sentence on the next line, then).
      ;;("[.:]\\s-+\\(\\sw+\\)\\s-*$" (1 font-lock-warning-face t))
      ;; Change Log entries should use present tense.
      ("):[ \t\n]*[[:alpha:]]+\\(ed\\)\\>"
       (1 '(face font-lock-warning-face help-echo "Use present tense.") t))
      ;; Change log entries start with a capital letter.
      ("): [a-z]" (0 '(face font-lock-warning-face help-echo "Capitalize.") t))
      ("[^[:upper:]]\\(\\. [[:upper:]]\\)"
       (1 '(face font-lock-warning-face
            help-echo "Use two spaces to end a sentence") t))
      ("^("
       (0 (let ((beg (max (point-min) (- (match-beginning 0) 2))))
            (put-text-property beg (match-end 0) 'font-lock-multiline t)
            (if (eq (char-syntax (char-after beg)) ?w)
                '(face font-lock-warning-face
                  help-echo "Punctuate previous line.")))
          t))
      ))

bug#68691: 30.0.50; [WISHLIST] Make it easier to conform to desired commit message format

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> It cannot.  Format could be okay, but will not describe all of the
> changes, or describe them in a nonsensical way.
>
> OTOH, is the format below OK or not?
>
>      Merge from origin/emacs-29
>
>      53481cc9546 Fix description of when "\xNNN" is considered a unibyte character
>      1ef8b90ae06 Simplify imenu setup for {cmake,dockerfile}-ts-modes
>      7338af9c986 ; * etc/PROBLEMS: Document that GnuPG 2.4.4 solves the EasyPG
>      5483a1df99c Improve documentation of profiler commands
>      fb4cf0ab46d ; Fix xref under Output Overrides in Elisp manual.
>      aa6c24da61f Fix broken links to Freedesktop notifications spec
>      14d68221d26 Fix nasty cut'n'waste error in Tramp
>      51ca049608c Fix image-dired-tags-db-file void variable error
>      c450eec07ff typescript-ts-mode: Skip test if tsx grammar missing
>      9841ced147f ; Fix typos
>      557ed9c0463 * admin/README: Document the run-codespell script.
>      5701f96335c * admin/README: Fix entry on coccinelle subdirectory.
>      1805f4bfd62 Add script admin/run-codespell and supporting files

This starts with "Merge from..."

> Or this one:
>
>     Revert "Fix typo in lispref 'Creating Strings' section"
>
>     This reverts commit b825962ea840348bbde0c834ca398458a06fbb8b
>     which was mistakenly installed in master instead of emacs-29.

This starts with the word "Revert"

> Or this one:
>
>     ; Update copyright years in more files

And this starts with a semicolon.

> How do you distinguish between the above perfectly valid examples, and
> some arbitrary text in a log message (which should be flagged as
> nonconforming)?

The types of commit messages that don't require log messages is finite,
so it is possible to explicitly permit them, I think.  I haven't tried
yet.

bug#68691: 30.0.50; [WISHLIST] Make it easier to conform to desired commit message format

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: stefankangas@gmail.com,  iarchivedmywholelife@gmail.com,
>   68691@debbugs.gnu.org
> Date: Sun, 28 Jan 2024 17:04:30 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > It cannot.  Format could be okay, but will not describe all of the
> > changes, or describe them in a nonsensical way.
> >
> > OTOH, is the format below OK or not?
> >
> >      Merge from origin/emacs-29
> >
> >      53481cc9546 Fix description of when "\xNNN" is considered a unibyte character
> >      1ef8b90ae06 Simplify imenu setup for {cmake,dockerfile}-ts-modes
> >      7338af9c986 ; * etc/PROBLEMS: Document that GnuPG 2.4.4 solves the EasyPG
> >      5483a1df99c Improve documentation of profiler commands
> >      fb4cf0ab46d ; Fix xref under Output Overrides in Elisp manual.
> >      aa6c24da61f Fix broken links to Freedesktop notifications spec
> >      14d68221d26 Fix nasty cut'n'waste error in Tramp
> >      51ca049608c Fix image-dired-tags-db-file void variable error
> >      c450eec07ff typescript-ts-mode: Skip test if tsx grammar missing
> >      9841ced147f ; Fix typos
> >      557ed9c0463 * admin/README: Document the run-codespell script.
> >      5701f96335c * admin/README: Fix entry on coccinelle subdirectory.
> >      1805f4bfd62 Add script admin/run-codespell and supporting files
> 
> This starts with "Merge from..."

So any message that begins with "Merge from" will be okay, no matter
what follows it?

> > Or this one:
> >
> >     Revert "Fix typo in lispref 'Creating Strings' section"
> >
> >     This reverts commit b825962ea840348bbde0c834ca398458a06fbb8b
> >     which was mistakenly installed in master instead of emacs-29.
> 
> This starts with the word "Revert"

So any message that begins with "Revert" will be okay, no matter what
follows it?

> > Or this one:
> >
> >     ; Update copyright years in more files
> 
> And this starts with a semicolon.

So any message with a semicolon is okay?

And if I use the same log, but without a semicolon (for whatever
reasons), then the hook will abort the commit?

> > How do you distinguish between the above perfectly valid examples, and
> > some arbitrary text in a log message (which should be flagged as
> > nonconforming)?
> 
> The types of commit messages that don't require log messages is finite,

No, it's infinite, actually.  The 3 examples I've shown are just those
I found in a few seconds I had.