GNU bug report logs - #24939
[PATCH] Add tests for lisp/kmacro.el

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 24939 in the body.
You can then email your comments to 24939 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Gemini Lasswell <gazally <at> runbox.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Add tests for lisp/kmacro.el
Date: Sun, 13 Nov 2016 13:23:51 -0800

[Message part 1 (text/plain, inline)]

Hello,
There weren't any tests for kmacro.el, so I have written some.

Two things that may yet need to be done:

1. These tests make extensive use of two macros, called
kmacro-tests-should-call and kmacro-tests-should-not-call. They are
context-creating macros which add advice to named functions for the
duration of a test. I think that these two macros would be a useful
addition to ERT, and I'll submit that idea as a separate patch.

2. I found several minor bugs in the process of writing these, leading
to tests marked as :expected-result :failed. One is a way to create an
empty keyboard macro using the mouse and the rest are ways to get
kmacro-step-edit-macro to behave oddly. I haven't sent them to
bug-gnu-emacs yet. When I do so would it be better to send them
individually or put all the step-edit ones in one report?

My copyright assignment paperwork was finished as of Nov 2.

[0001-Add-tests-for-lisp-kmacro.el.patch (text/plain, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Gemini Lasswell <gazally <at> runbox.com>
Cc: 24939 <at> debbugs.gnu.org
Subject: Re: bug#24939: [PATCH] Add tests for lisp/kmacro.el
Date: Mon, 14 Nov 2016 17:49:47 +0200

> From: Gemini Lasswell <gazally <at> runbox.com>
> Date: Sun, 13 Nov 2016 13:23:51 -0800
> 
> There weren't any tests for kmacro.el, so I have written some.

Thanks!

> 1. These tests make extensive use of two macros, called
> kmacro-tests-should-call and kmacro-tests-should-not-call. They are
> context-creating macros which add advice to named functions for the
> duration of a test. I think that these two macros would be a useful
> addition to ERT, and I'll submit that idea as a separate patch.

Thanks, I respond to this point there.

> 2. I found several minor bugs in the process of writing these, leading
> to tests marked as :expected-result :failed. One is a way to create an
> empty keyboard macro using the mouse and the rest are ways to get
> kmacro-step-edit-macro to behave oddly. I haven't sent them to
> bug-gnu-emacs yet. When I do so would it be better to send them
> individually or put all the step-edit ones in one report?

It's a judgment call.  If the offending function is small enough (it
is in this case), and the required changes aren't expected to be too
complex, then one report is better.

Allow me a few comments to your proposed patch.

> +(defmacro kmacro-tests-should-not-call (func-or-funcs &rest body)
> +  "Verify that a function or functions are not called during execution.
> +FUNC-OR-FUNCS can either be a single function or a list of them.
> +Temporarily override them them during the execution of BODY with advice
                        ^^^^^^^^^
One "them" is redundant.

> +containing `should-not' and a non-nil value.  Use the function symbol
> +as the non-nil value to make tracking down what went wrong a bit
> +faster."

A general comment: I think your doc strings describe the
implementation too intimately.  I appreciate the effort that went into
doing that, but the result has 2 disadvantages: (1) the doc string
needs much more maintenance than it should, because any change to the
implementation will need the doc string to be updated (which runs a
high risk of the doc becoming out of sync), and (2) the user of the
function needs to wade through details she doesn't really need to
know.

A doc string should describe the arguments, the return value, and the
side effects, if any, of the function, without going into the
implementation.

> +(defmacro kmacro-tests-should-insert (value &rest body)
> +  "Verify that VALUE is inserted by the execution of BODY.
> +Execute BODY, then check that the string VALUE was inserted
> +into the current buffer at point.  As a side effect captures the
> +symbols p_ and bsize_."

Without explaining what p_ and bsize_ are, the last sentence of the
doc string are not really useful, IMO.

> +  (declare (debug (stringp body))
> +           (indent 1))
> +  `(let ((p_ (point))
> +         (bsize_ (buffer-size)))
> +     ,@body
> +     (should (equal (buffer-substring p_ (point)) ,value))
> +     (should (equal (- (buffer-size) bsize_) (string-width ,value)))))
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This doesn't look right to me: string-width takes the char-width of
each character into account, so it cannot be reliably compared to
buffer size.  I think you want to call length instead.

> +(defmacro kmacro-tests-should-match-message (value &rest body)
> +  "Verify that a message matching VALUE is issued while executing BODY.
> +Execute BODY, then check for a regexp match between
> +VALUE and any text written to *Messages* during the execution."
> +  (declare (debug (stringp body))
> +           (indent 1))
> +  `(with-current-buffer (get-buffer-create "*Messages*")
> +     (save-restriction
> +       (narrow-to-region (point-max) (point-max))
> +       ,@body
> +       (should (string-match-p ,value (buffer-string))))))

I don't like this implementation.  First, playing restriction games
with *Messages* is inherently unsafe, because that buffer is treated
specially by the code which puts messages there.  Second, this assumes
*Messages* will have each message verbatim, which is false, because
repeated messages aren't inserted.  And finally, some code can disable
message logging or use some mechanism for displaying echo-area
messages that bypasses *Messages*, in which case this macro will not
catch the message.

So I'd suggest instead to override or advice 'message', so you could
get your hands on the messages more reliably.  It is possible we
should have a more thorough infrastructure for collecting echo-area
messages, which probably means parts of it should be implemented in C,
but that's a separate project.

> +(kmacro-tests-deftest kmacro-tests-test-insert-counter-01-nil ()
> +  "Insert counter adds one with nil arg."
      ^^^^^^^^^^^^^^
I think you meant "Inserted counter", here and elsewhere.

> +(kmacro-tests-deftest kmacro-tests-test-start-macro-when-defining-macro ()
> +  "Starting a macro while defining a macro does not start a second macro."
> +  ;; Verify kmacro-start-macro calls start-kbd-macro
> +  (:stub start-kbd-macro)
> +  (kmacro-tests-should-call
> +      ((start-kbd-macro :once :before (lambda (append noexec)
> +                                        (should-not append)
> +                                        (should-not noexec))))
> +    (kmacro-tests-with-current-prefix-arg (kmacro-start-macro nil)))
> +  ;; And leaves macro in being-recorded state
> +  (should defining-kbd-macro)
> +  ;; And that it doesn't call it again
> +  (kmacro-tests-should-not-call start-kbd-macro
> +    (kmacro-tests-with-current-prefix-arg (kmacro-start-macro nil))))

I have a grave issue with this kind of testing: it is too tied to the
implementation, instead of testing the behavior and the effects.
E.g., the fact that start-kbd-macro is only called once isn't a
reliable evidence that "starting a macro while defining a macro does
not start a second macro"; a reliable evidence would be to verify that
the second macro isn't defined after the form finishes.

I think there's a fundamental issue here, regarding the purpose of our
test suite and consequently the methodology we should use for writing
the tests.

More about this in my response to your proposed ert-should-call and
ert-should-not-call additions.  Here I just note that most of your
tests are based on this paradigm, which I think makes these tests much
more complex and fragile than they need to be.

> +(kmacro-tests-deftest kmacro-tests-set-macro-counter-while-defining ()
> +  "Use of the prefix arg with kmacro-start sets kmacro-counter."
> +  (:stub start-kbd-macro)
> +  ;; Give kmacro-start-macro an argument
> +  (kmacro-tests-should-call
> +      ((start-kbd-macro :once :before (lambda (append noexec)
> +                                        (should-not append)
> +                                        (should-not noexec))))
> +    (kmacro-tests-with-current-prefix-arg (kmacro-start-macro 5)))
> +  (should defining-kbd-macro)
> +  ;; And verify that the counter is set to that value
> +  (ert-with-test-buffer (:name "counter-while-defining")
> +    (kmacro-tests-should-insert "5"
> +      (kmacro-tests-with-current-prefix-arg (kmacro-insert-counter nil)))
> +    ;; change it while defining macro
> +    (kmacro-tests-with-current-prefix-arg (kmacro-set-counter 1))
> +    (kmacro-tests-should-insert "1"
> +      (kmacro-tests-with-current-prefix-arg (kmacro-insert-counter nil)))
> +    ;; using universal arg to to set counter should reset to starting value

Please use a consistent style in comments: begin with a capital letter
and end with a period.  These are our code conventions.

> +      (message "")  ; clear the echo area
> +      (kmacro-tests-should-match-message "Type e to repeat macro"

Why is that call to 'message' necessary here?  I suspect this is one
symptom of the fragility of kmacro-tests-should-match-message that I
mentioned above.

> +(kmacro-tests-deftest kmacro-tests-test-ring-commands-when-no-macros ()
> +  "Ring commands give appropriate message when no macros exist."
> +  (dolist (cmd '((kmacro-cycle-ring-next nil)
> +                 (kmacro-cycle-ring-previous nil)
> +                 (kmacro-swap-ring)
> +                 (kmacro-delete-ring-head)
> +                 (kmacro-view-ring-2nd)
> +                 (kmacro-call-ring-2nd nil)
> +                 (kmacro-view-macro)))
> +    (kmacro-tests-should-match-message "No keyboard macro defined"
> +      (apply #'funcall cmd))))
         ^^^^^^^^^^^^^^^^^^^^^
Why not ert-simulate-command?

> +  ;; I'm attempting to make the test work even if keys have been
> +  ;; rebound, but if this is failing try emacs -Q first.

If this comment is still valid, then many other parts of the test have
the same problem, because they clearly assume the default key
bindings.  If these tests are to be run as part of the test suite in
"emacs -batch" or "emacs -Q", then these problems don't exist; but if
not, your "clean slate" should somehow restore the default key
bindings, or else call the functions by name.

Thanks again for working on this.

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

From: Gemini Lasswell <gazally <at> runbox.com>
To: Eli Zaretskii <eliz <at> gnu.org>, 24939 <at> debbugs.gnu.org
Subject: Re: bug#24939: [PATCH] Add tests for lisp/kmacro.el
Date: Mon, 14 Nov 2016 10:26:42 -0800

Thanks for the thorough feedback! I'll work on a new & improved version
incorporating your comments.
Some responses/questions below.

>> +(defmacro kmacro-tests-should-match-message (value &rest body)
>> +  "Verify that a message matching VALUE is issued while executing BODY.
>> +Execute BODY, then check for a regexp match between
>> +VALUE and any text written to *Messages* during the execution."
>> +  (declare (debug (stringp body))
>> +           (indent 1))
>> +  `(with-current-buffer (get-buffer-create "*Messages*")
>> +     (save-restriction
>> +       (narrow-to-region (point-max) (point-max))
>> +       ,@body
>> +       (should (string-match-p ,value (buffer-string))))))
>
> I don't like this implementation.

This strategy is used in autorevert-tests.el and filenotify-tests.el,
which is where I copied it from. So those should be changed too.

> get your hands on the messages more reliably.  It is possible we
> should have a more thorough infrastructure for collecting echo-area
> messages, which probably means parts of it should be implemented in C,
> but that's a separate project.

That would definitely be helpful.

>> +      (message "")  ; clear the echo area
>> +      (kmacro-tests-should-match-message "Type e to repeat macro"
>
> Why is that call to 'message' necessary here?  I suspect this is one
> symptom of the fragility of kmacro-tests-should-match-message that I
> mentioned above.

Actually not. I was attempting to write a regression for bug#11817,
which was about the "Type e to repeat macro" not showing up in the echo
area. But kmacro-call-macro doesn't put up the message if there's
already a message. I just looked into the history of that bit of code,
and kmacro-call-macro's check of (current-message) is actually another
bug fix, for bug#3412.

>> +(kmacro-tests-deftest kmacro-tests-test-ring-commands-when-no-macros ()
>> +  "Ring commands give appropriate message when no macros exist."
>> +  (dolist (cmd '((kmacro-cycle-ring-next nil)
>> +                 (kmacro-cycle-ring-previous nil)
>> +                 (kmacro-swap-ring)
>> +                 (kmacro-delete-ring-head)
>> +                 (kmacro-view-ring-2nd)
>> +                 (kmacro-call-ring-2nd nil)
>> +                 (kmacro-view-macro)))
>> +    (kmacro-tests-should-match-message "No keyboard macro defined"
>> +      (apply #'funcall cmd))))
>          ^^^^^^^^^^^^^^^^^^^^^
> Why not ert-simulate-command?

I'll make that change. What do you think about changing
ert-simulate-command to set current-prefix-arg? That would be very
helpful.

>> +  ;; I'm attempting to make the test work even if keys have been
>> +  ;; rebound, but if this is failing try emacs -Q first.
>
> If this comment is still valid, then many other parts of the test have
> the same problem, because they clearly assume the default key
> bindings.

That comment is out of date, since kmacro-tests-keymap should fix the
problem. I'll remove it.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Gemini Lasswell <gazally <at> runbox.com>
Cc: 24939 <at> debbugs.gnu.org
Subject: Re: bug#24939: [PATCH] Add tests for lisp/kmacro.el
Date: Mon, 14 Nov 2016 20:47:23 +0200

> From: Gemini Lasswell <gazally <at> runbox.com>
> Cc: 
> Date: Mon, 14 Nov 2016 10:26:42 -0800
> 
> >> +(defmacro kmacro-tests-should-match-message (value &rest body)
> >> +  "Verify that a message matching VALUE is issued while executing BODY.
> >> +Execute BODY, then check for a regexp match between
> >> +VALUE and any text written to *Messages* during the execution."
> >> +  (declare (debug (stringp body))
> >> +           (indent 1))
> >> +  `(with-current-buffer (get-buffer-create "*Messages*")
> >> +     (save-restriction
> >> +       (narrow-to-region (point-max) (point-max))
> >> +       ,@body
> >> +       (should (string-match-p ,value (buffer-string))))))
> >
> > I don't like this implementation.
> 
> This strategy is used in autorevert-tests.el and filenotify-tests.el,
> which is where I copied it from. So those should be changed too.

Most probably.  But let's first see what better implementation we
could come up with.

> > get your hands on the messages more reliably.  It is possible we
> > should have a more thorough infrastructure for collecting echo-area
> > messages, which probably means parts of it should be implemented in C,
> > but that's a separate project.
> 
> That would definitely be helpful.

If you can provide a reasonable spec for such a feature, I'm sure
Someone™ will make it happen.

> >> +    (kmacro-tests-should-match-message "No keyboard macro defined"
> >> +      (apply #'funcall cmd))))
> >          ^^^^^^^^^^^^^^^^^^^^^
> > Why not ert-simulate-command?
> 
> I'll make that change. What do you think about changing
> ert-simulate-command to set current-prefix-arg? That would be very
> helpful.

Sounds like a useful extension to me.

> >> +  ;; I'm attempting to make the test work even if keys have been
> >> +  ;; rebound, but if this is failing try emacs -Q first.
> >
> > If this comment is still valid, then many other parts of the test have
> > the same problem, because they clearly assume the default key
> > bindings.
> 
> That comment is out of date, since kmacro-tests-keymap should fix the
> problem. I'll remove it.

OK, then there are a couple more such comments in the patch.

Thanks.

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

From: Gemini Lasswell <gazally <at> runbox.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 24939 <at> debbugs.gnu.org
Subject: Re: bug#24939: [PATCH] Add tests for lisp/kmacro.el
Date: Tue, 29 Nov 2016 12:56:36 -0800

Eli Zaretskii <eliz <at> gnu.org> writes:

>> >> +(defmacro kmacro-tests-should-match-message (value &rest body)
>> >
>> > I don't like this implementation.
>>
>> This strategy is used in autorevert-tests.el and filenotify-tests.el,
>> which is where I copied it from. So those should be changed too.
>
> Most probably.  But let's first see what better implementation we
> could come up with.
>

Here is an implementation of message capturing using advice:

(defmacro ert-with-message-capture (var &rest body)
  "Execute BODY while collecting messages in VAR.
Capture all messages produced by `message' when it is called from
Lisp, and concatenate them separated by newlines into one string."
  (declare (debug (symbolp body))
           (indent 1))
  (let ((g-advice (cl-gensym)))
    `(let* ((,var "")
            (,g-advice (lambda (func &rest args)
                         (if (or (null args) (equal (car args) ""))
                             (apply func args)
                           (let ((msg (apply #'format-message args)))
                             (setq ,var (concat ,var msg "\n"))
                             (funcall func "%s" msg))))))
       (advice-add 'message :around ,g-advice)
       (unwind-protect
           (progn ,@body)
         (advice-remove 'message ,g-advice)))))

The reason I have set it up to bind a variable during the code body
execution is to make it usable by autorevert-tests.el and
filenotify-tests.el. For example, here's an excerpt from
filenotify-tests.el:

(with-current-buffer (get-buffer-create "*Messages*")
  (narrow-to-region (point-max) (point-max)))
(sleep-for 1)
(write-region
 "another text" nil file-notify--test-tmpfile nil 'no-message)

;; Check, that the buffer has been reverted.
(with-current-buffer (get-buffer-create "*Messages*")
  (file-notify--wait-for-events
   timeout
   (string-match
    (format-message "Reverting buffer `%s'." (buffer-name buf))
    (buffer-string))))

file-notify--wait-for-events polls read-event in a loop with a
fractional timeout until the form that is its second argument becomes
non-nil, or the larger timeout expires. With the macro above, this
excerpt becomes:

(ert-with-message-capture captured-messages
  (sleep-for 1)
  (write-region
   "another text" nil file-notify--test-tmpfile nil 'no-message)

  ;; Check, that the buffer has been reverted.
  (file-notify--wait-for-events
     timeout
     (string-match
      (format-message "Reverting buffer `%s'." (buffer-name buf))
      captured-messages)))

Let me know what you think.

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Gemini Lasswell <gazally <at> runbox.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 24939 <at> debbugs.gnu.org
Subject: Re: bug#24939: [PATCH] Add tests for lisp/kmacro.el
Date: Wed, 30 Nov 2016 10:08:46 +0100

Gemini Lasswell <gazally <at> runbox.com> writes:

Hi,

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> > I don't like this implementation.
>>>
>>> This strategy is used in autorevert-tests.el and filenotify-tests.el,
>>> which is where I copied it from. So those should be changed too.
>>
>> Most probably.  But let's first see what better implementation we
>> could come up with.
>>
> Let me know what you think.

At first glance, looks OK to me. However, I didn't understand why the
existing implementation (watching messages in *Messages*) is bad.

Eli?

Best regards, Michael.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: gazally <at> runbox.com, 24939 <at> debbugs.gnu.org
Subject: Re: bug#24939: [PATCH] Add tests for lisp/kmacro.el
Date: Wed, 30 Nov 2016 17:51:17 +0200

> From: Michael Albinus <michael.albinus <at> gmx.de>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  24939 <at> debbugs.gnu.org
> Date: Wed, 30 Nov 2016 10:08:46 +0100
> 
> > Eli Zaretskii <eliz <at> gnu.org> writes:
> >
> >>> > I don't like this implementation.
> >>>
> >>> This strategy is used in autorevert-tests.el and filenotify-tests.el,
> >>> which is where I copied it from. So those should be changed too.
> >>
> >> Most probably.  But let's first see what better implementation we
> >> could come up with.
> >>
> > Let me know what you think.
> 
> At first glance, looks OK to me.

I agree.  Thanks for working on this, Gemini.

> However, I didn't understand why the existing implementation
> (watching messages in *Messages*) is bad.

Because the display engine treats that buffer specially, and assumes
all kinds of assumptions when it does.  See message_dolog in xdisp.c
for the gory details.

So I'd like us to futz as little as possible with *Messages*, lest we
violate those assumptions.  In particular, setting buffer restrictions
might get in the way.  (Granted, this is only a problem when the test
suite is run interactively, but AFAIK this is actually done
sometimes.)

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: gazally <at> runbox.com, 24939 <at> debbugs.gnu.org
Subject: Re: bug#24939: [PATCH] Add tests for lisp/kmacro.el
Date: Wed, 30 Nov 2016 17:51:19 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

>> However, I didn't understand why the existing implementation
>> (watching messages in *Messages*) is bad.
>
> Because the display engine treats that buffer specially, and assumes
> all kinds of assumptions when it does.  See message_dolog in xdisp.c
> for the gory details.
>
> So I'd like us to futz as little as possible with *Messages*, lest we
> violate those assumptions.  In particular, setting buffer restrictions
> might get in the way.  (Granted, this is only a problem when the test
> suite is run interactively, but AFAIK this is actually done
> sometimes.)

I didn't know this, thanks for the explanation. So I'm OK with the change.

Best regards, Michael.

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

From: Gemini Lasswell <gazally <at> runbox.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 24939 <at> debbugs.gnu.org, Michael Albinus <michael.albinus <at> gmx.de>
Subject: Re: bug#24939: [PATCH] Add tests for lisp/kmacro.el
Date: Sat, 10 Dec 2016 09:46:03 -0800

I've submitted a patch to add the message capturing macro to ert-x.el as
a separate issue, #25158.

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

From: Gemini Lasswell <gazally <at> runbox.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 24939 <at> debbugs.gnu.org
Subject: Re: bug#24939: [PATCH] Add tests for lisp/kmacro.el
Date: Sat, 31 Dec 2016 09:42:25 -0800

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:
> Allow me a few comments to your proposed patch.

Here is an updated version taking your comments into account. I was able
to remove the use of advice on all the functions in macros.c except for
end-kbd-macro, because I couldn't come up with another way to make
kmacro-end-macro recognize a keyboard macro defined by a test.
kmacro-end-macro tests last-kbd-macro right after it calls
end-kbd-macro, and end-kbd-macro is going to leave last-kbd-macro empty
unless it has something in current_kboard->kbd_macro_buffer, which Lisp
can't access. If there is another strategy you'd like me to try there,
let me know.

This patch contains kmacro-tests-with-message-capture which is the same
as the macro proposed as an addition to ert-x.el in bug#25158, so if
that patch is adopted, it could be removed from this patch.

[0001-Add-tests-for-lisp-kmacro.el.patch (text/plain, attachment)]

Message #37 received at 24939-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Gemini Lasswell <gazally <at> runbox.com>
Cc: 24939-done <at> debbugs.gnu.org
Subject: Re: bug#24939: [PATCH] Add tests for lisp/kmacro.el
Date: Sat, 04 Feb 2017 13:57:16 +0200

> From: Gemini Lasswell <gazally <at> runbox.com>
> Cc: 24939 <at> debbugs.gnu.org
> Date: Sat, 31 Dec 2016 09:42:25 -0800
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> > Allow me a few comments to your proposed patch.
> 
> Here is an updated version taking your comments into account. I was able
> to remove the use of advice on all the functions in macros.c except for
> end-kbd-macro, because I couldn't come up with another way to make
> kmacro-end-macro recognize a keyboard macro defined by a test.
> kmacro-end-macro tests last-kbd-macro right after it calls
> end-kbd-macro, and end-kbd-macro is going to leave last-kbd-macro empty
> unless it has something in current_kboard->kbd_macro_buffer, which Lisp
> can't access. If there is another strategy you'd like me to try there,
> let me know.

Thanks, I pushed this now.

> This patch contains kmacro-tests-with-message-capture which is the same
> as the macro proposed as an addition to ert-x.el in bug#25158, so if
> that patch is adopted, it could be removed from this patch.

I've replaced the macro with ert-with-message-capture.

Previous Next

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