[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#61549: 30.0.50; [PATCH] New keyboard macro counter functions
From: |
Eli Zaretskii |
Subject: |
bug#61549: 30.0.50; [PATCH] New keyboard macro counter functions |
Date: |
Fri, 17 Feb 2023 10:13:45 +0200 |
> 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.