GNU bug report logs - #61549
30.0.50; [PATCH] New keyboard macro counter functions

Previous Next

Full log

Message #8 received at 61549 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alex Bochannek <alex <at> bochannek.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 61549 <at> debbugs.gnu.org
Subject: Re: bug#61549: 30.0.50; [PATCH] New keyboard macro counter functions
Date: Fri, 17 Feb 2023 10:13:45 +0200

> From: Alex Bochannek <alex <at> 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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.