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

[Eli Zaretskii <eliz@gnu.org>]

> From: Alex Bochannek <alex@bochannek.com>
> Date: Thu, 16 Feb 2023 00:17:25 -0800
> 
> I have been working on blog posts about keyboard macros and found that
> it would be useful to have comparison functions for the keyboard macro
> counter.

Thanks.

> I implemented two functions to load and save macro counter values from
> and to number registers; three comparison functions of the macro counter
> with a number register that conditionally increment the counter; three
> comparison functions of the macro counter with a prefix that terminate
> the macro execution. This simplifies handling multiple counters and
> conditional macro termination.
> 
> I am attaching the changes to:
>   emacs.texi
>   kmacro.texi
>   NEWS
>   kmacro.el
>   kmacro-tests.el
> 
> I hope this functionality is useful and that I followed the coding and
> style standards.

I wonder whether these commands are important enough to have them in
the manual.  Stefan and Lars, WDYT?  Any other comments to the feature
and its implementation?

> +(defun kmacro-reg-load-counter (register)
> +  "Load the value of a register into `kmacro-counter'"

The first line of a doc string should be a single complete sentence,
ending with a period (here and elsewhere).  You may wish running
checkdoc on your code to reveal any issues.

> +(defun kmacro-reg-add-counter-equal (&optional arg)
> +  "Increment counter by ARG if it is equal to register value"

This doc string is confusing, I think.  Would you like to reword it to
clarify whet the command does?  In particular, the "it" part is
ambiguous.

> +(defun kmacro-reg-add-counter-less (&optional arg)
> +  "Increment counter by ARG if it is less than register value"
> +  (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)
> +  "Increment counter by ARG if it is greater than register value"
> +  (interactive "p")
> +  (let
> +      ((register (register-read-with-preview "Compare counter to register: ")))
> +    (kmacro-reg-add-counter '> register arg)))

Similar problems with the doc strings of these two commands.

> +(defun kmacro-reg-add-counter (func register &optional arg)
> +  "Increment the counter by ARG if (FUNC kmacro-counter REGISTER-VALUE)
> +is true.
> +With no ARG, ARG is set to 1"

Our style is to say "ARG is the numeric prefix argument that defaults
to 1."

> +(defun kmacro-quit-counter-equal (&optional arg)
> +  "Quit the keyboard macro if the counter is equal to ARG"

"when the counter is equal to ARG", I guess?

> +(defun kmacro-quit-counter-less (&optional arg)
> +  "Quit the keyboard macro if the counter is less than ARG"
> +  (interactive "P")
> +  (kmacro-quit-counter '< arg))
> +
> +
> +(defun kmacro-quit-counter-greater (&optional arg)
> +  "Quit the keyboard macro if the counter is greater than ARG"
> +    (interactive "P")
> +    (kmacro-quit-counter '> arg))

Likewise here.

> +(defun kmacro-quit-counter (func &optional arg)
> +  "Quit the keyboard macro if (FUNC kmacro-counter ARG) is true.

Our style is to use PRED instead of FUNC, and document like this:

    Quit the keyboard macro when predicate PRED returns non-nil.
  PRED is called with two arguments: kmacro-counter and ARG.

> +With \\[universal-argument] or no ARG, ARG is set to 0"

  "Arg is the prefix numeric argument and defaults to zero."

> +  (let ((arg
> +	 (cond ((or (consp arg) (null arg)) 0)
> +	       ((eq '- arg) -1)
> +	       (t arg))))

This seems to imply that ARG has meaning beyond what the above text
says.