Subject: bug#44202: [PATCH] Add variable to control confirmation of help-mode-revert-buffer

From: Stefan Kangas <[email-masked]>
Date: Sat, 24 Oct 2020 17:01:19 -0700

Kevin Foley <[email-masked]> writes:

> Allows users to control the `noconfirm' parameter of
> `help-mode-revert-buffer'.
>
> I've assigned copyright to FSF but this is my first patch on this list
> so please let me know if I've missed any standards.

Thanks, it looks useful.  I have added my comments below.

Your patch seems to be lacking ChangeLog entries as described in
etc/CONTRIBUTE.  You would help us if you could add such entries,
especially if you are planning to do more changes.

> +(defcustom help-mode-revert-buffer-noconfirm nil

This would change the default, right?  Any rationale for that?

Perhaps we could keep this new behavior optional for now?  We could
consider changing it later once we get some experience with it.

> +  "Indicates whether to prompt for confirmation when reverting a
> +help buffer."

The first sentence of a doc string should fit on one line.

Perhaps you could use something like:

    "If non-nil, prompt before reverting a help buffer."

>  (defun help-mode-revert-buffer (_ignore-auto noconfirm)
> -  (when (or noconfirm (yes-or-no-p "Revert help buffer? "))
> +  "Revert help-mode buffer.  See
> +`help-mode-revert-buffer-noconfirm' to control whether user is
> +prompted for confirmation."

The first line should be one sentence only, and the second sentence
start on the second line.

Other than those nits, the patch looks good to me.