bug#61549: 30.0.50; [PATCH] New keyboard macro counter functions

[Eli Zaretskii]

> From: Alex Bochannek <alex@bochannek.com>
> Cc: monnier@iro.umontreal.ca,  larsi@gnus.org,  61549@debbugs.gnu.org
> Date: Sun, 05 Mar 2023 19:37:21 -0800
> 
> Took me a little bit longer to find time to do this and I now have
> incorporated your feedback in the below patch. Thank you for your
> perspective on prefix, that made a lot of sense and I reworked that part
> of the code to be consistent with how it usually works. I was not aware
> that the interactive code `p' defaults to 1 in the absence of a prefix.
> I couldn't find a place where this is documented and it simplified the
> code.
> 
> I updated the docstrings as you suggested and even though checkdoc
> complained about the lack of a period on the first line, I figured it's
> better to keep below the line length limits.

Thanks, a few more comments below.

> Let me know if you would like to see any other changes, I always
> appreciate constructive feedback!
> 
> I am attaching the changes to:
>   kmacro.texi
>   kmacro.el
>   kmacro-tests.el
> 
> The changelog as well as NEWS and emacs.texi remain the same from my
> original message.

Stefan and Lars didn't respond, and I tend to think there's no need to
describe these functions in the Emacs user manual.  So for the next
iteration (which hopefully will be the last), please submit the patch
without the changes in the manual.  Also, please post all of the
other changes, including NEWS and the commit log message (and mention
the bug number in the latter).

> +(defun kmacro-reg-add-counter-equal (&optional arg)
> +  "Increment `kmacro-counter' by ARG if the counter is equal to a
> +register's value.

The first line of a doc string must be a complete sentence.

> +(defun kmacro-reg-add-counter-less (&optional arg)
> +  "Increment `kmacro-counter' by ARG if the counter is less than a
> +register's value.

Likewise here (and elsewhere in the patch).

> +ARG is the numeric prefix argument that defaults to one."
> +  (interactive "p")
> +  (let
> +      ((register (register-read-with-preview "Compare counter to register: 
> ")))
> +    (kmacro-reg-add-counter '< register arg)))
> +
> +
> +(defun kmacro-reg-add-counter-greater (&optional arg)

I noticed that you always leave 2 empty lines between functions.  Is
that intentional?  We generally leave just one empty line.

> +(defun kmacro-reg-add-counter (pred register arg)
> +  "Increment `kmacro-counter' by ARG if predicate PRED returns
> +non-nil.
> +PRED is called with two arguments: `kmacro-counter' and REGISTER."
> +  (let ((register-val (get-register register)))
> +    (when (apply pred (list kmacro-counter register-val))
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
To avoid consing a list here, would it be better to use funcall
instead of apply here?

> +(defun kmacro-quit-counter (pred &optional arg)
> +  "Quit the keyboard macro if predicate PRED returns non-nil.
> +PRED is called with two arguments: `kmacro-counter' and ARG."
> +  (when kmacro-initial-counter-value
> +    (setq kmacro-counter kmacro-initial-counter-value
> +       kmacro-initial-counter-value nil))
> +  (let ((arg
> +      (cond ((null arg) 0)
> +            (t (prefix-numeric-value arg)))))
> +    (when (apply pred (list kmacro-counter arg))
> +      (keyboard-quit))))

Likewise here.