Subject: bug#24880: 26.0.50; buff-menu: Command to unmark all buffers

From: Eli Zaretskii <[email-masked]>
Date: Tue, 15 Nov 2016 18:03:35 +0200

> From: Tino Calancha <[email-masked]>
> Date: Mon, 14 Nov 2016 19:29:14 +0900 (JST)
> cc: Tino Calancha <[email-masked]>, Eli Zaretskii <[email-masked]>, 
>     [email-masked]
> 
> This is the amended patch:

Thanks, please see a few comments below.

> +@item M-@key{DEL} @var{markchar}

Please remove the @var{markchar} part, we don't usually show user
input after the keys.

> +(defun bs-unmark-previous (count)
> +  "Unmark previous COUNT buffers.
> +Move cursor vertically up COUNT lines."
> +  (interactive "p")

The doc string should describe the effect of the prefix argument in
interactive usage.

Also, we prefer to say "move point", not "move cursor", since that's
what the commands do; cursor movement is then performed by redisplay
as the side effect.

> +    (define-key map "\M-\C-?" 'Buffer-menu-unmark-all-buffers)

I'd prefer to use "\M-\177" instead, for consistency with DEL.  (Yes,
I know that other packages use \C-?, but we must start somewhere.)

> +    (bindings--define-key menu-map [umkab]
> +      '(menu-item "Unmark all buffers with mark" Buffer-menu-unmark-all-buffers

Labels of menu items which invoke commands that prompt user for input
should end in "...", as an indication that user input will be
required.

Also, "unmark ... with mark" sounds confusing, and is also quite
long.  How about "Remove marks..."?

> +                  :help "Cancel a requested operation on buffers"))
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This should say "all buffers", right?

> +(defun Buffer-menu-unmark-all-buffers (mark)
> +  "Cancel a requested operation on all buffers."
> +  (interactive "cRemove marks (RET means all):")

This doc string should mention the argument MARK, and should also tell
that the in interactive use the user is prompted for the character.