bug#28008: 25.2; Resume kmacro definition errors C-u C-u <F3>

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#28008: 25.2; Resume kmacro definition errors C-u C-u <F3>

Allen Li
Reproduction:

1. emacs -Q
2. F3
3. Type some stuff (asdfasdf)
4. C-g
5. C-u C-u F3

Expected: kmacro definition resumes

Actual: error

Debugger entered--Lisp error: (wrong-type-argument arrayp nil)
  start-kbd-macro(t t)
  kmacro-start-macro((16))
  kmacro-start-macro-or-insert-counter((16))
  funcall-interactively(kmacro-start-macro-or-insert-counter (16))
  call-interactively(kmacro-start-macro-or-insert-counter nil nil)
  command-execute(kmacro-start-macro-or-insert-counter)


In GNU Emacs 25.2.1 (x86_64-pc-linux-gnu, GTK+ Version 3.10.8),
modified by Debian
Windowing system distributor 'The X.Org Foundation', version 11.0.11803000
System Description: Ubuntu 14.04 LTS

Configured using:
 'configure --build x86_64-linux-gnu --build x86_64-linux-gnu
 --prefix=/usr --sharedstatedir=/var/lib --libexecdir=/usr/lib
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-pop=yes
 --with-crt-dir=/usr/lib/x86_64-linux-gnu --disable-build-details
 --disable-silent-rules --with-modules GOOGLE_VERSION=25.2+gg1+8
 --with-x=yes --with-x-toolkit=gtk3 --with-toolkit-scroll-bars
 build_alias=x86_64-linux-gnu 'CFLAGS=-g -O2 -fstack-protector
 --param=ssp-buffer-size=4 -Wformat -Werror=format-security -Wall'
 'LDFLAGS=-Wl,-Bsymbolic-functions -Wl,-z,relro
 -Wl,-fuse-ld=gold,--export-dynamic-symbol=__google_auxv'
 'CPPFLAGS=-D_FORTIFY_SOURCE=2 -DGOOGLE_EMACS_DEFINE_AUXV''

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF GSETTINGS
NOTIFY LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 MODULES



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#28008: 25.2; Resume kmacro definition errors C-u C-u <F3>

Tino Calancha-2
Allen Li <[hidden email]> writes:

> Reproduction:
>
> 1. emacs -Q
> 2. F3
> 3. Type some stuff (asdfasdf)
> 4. C-g
> 5. C-u C-u F3
>
> Expected: kmacro definition resumes
>
> Actual: error
>
> Debugger entered--Lisp error: (wrong-type-argument arrayp nil)
>   start-kbd-macro(t t)
Thank you for the report.
I)
  Isn't it this behavior expected?
  C-g ends `start-kbd-macro' before any macro has
  being saved; i.e., `last-kbd-macro' is nil, so we cannot append to it.

  You must have a saved macro to append:
  emacs -Q
  F3
  (insert "a") RET
  F4 ; save it in `last-kbd-macro'

  C-u C-u F3 ; Apped to it.
  (insert "b") RET
  F4 ; Save it.

  F4 ; This insert "ab" in the current buffer.

2)
  Expected or not, i think `kmacro-start-macro' might throw an error
  when the user wants to append and `start-kbd-macro' is nil.

--8<-----------------------------cut here---------------start------------->8---
commit 9c86eed0b015950a4ae06243c5807d9b864eb69f
Author: Tino Calancha <[hidden email]>
Date:   Tue Aug 8 14:14:55 2017 +0900

    Append kbd macro only if last-kbd-macro is non-nil
   
    * lisp/kmacro.el (kmacro-start-macro): Append only if
    last-kbd-macro is non-nil (Bug#28008).

diff --git a/lisp/kmacro.el b/lisp/kmacro.el
index 2db8061fa4..8eff7e5c2e 100644
--- a/lisp/kmacro.el
+++ b/lisp/kmacro.el
@@ -584,7 +584,8 @@ kmacro-start-macro
       kmacro-last-counter kmacro-counter
       kmacro-counter-format kmacro-default-counter-format
       kmacro-counter-format-start kmacro-default-counter-format))
-
+      (when (and append (null last-kbd-macro))
+        (user-error "No kbd macro has been defined"))
       (start-kbd-macro append
        (and append
     (if kmacro-execute-before-append
--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2017-08-08
Repository revision: c3445aed51944becb3e58f5dace8121c0021f6c7



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#28008: 25.2; Resume kmacro definition errors C-u C-u <F3>

Allen Li
That does make a kind of sense, but it seems to make the feature
useless for half the cases where you would want to use it.

I found others with the same use case asking the same question:
https://emacs.stackexchange.com/questions/3211/how-to-resume-an-incomplete-keyboard-macro-recording

Having any input error cause the macro recording to die makes
recording macros precarious.  One accidental typo, say C-x instead of
C-c, and you can't even C-g out of it, without having to start over or
take a detour through lossage.

I can understand a bare C-g canceling the macro definition, but why
not have errors save the macro definition?  I can't think of any case
where I would want an error to not save the macro I have been
painstakingly defining, but plenty of cases where I would want to
resume or edit a macro definition after an error.  Heck, there's the
macro ring if you really wanted the previous macro.

On Mon, Aug 7, 2017 at 10:26 PM, Tino Calancha <[hidden email]> wrote:

> Allen Li <[hidden email]> writes:
>
>> Reproduction:
>>
>> 1. emacs -Q
>> 2. F3
>> 3. Type some stuff (asdfasdf)
>> 4. C-g
>> 5. C-u C-u F3
>>
>> Expected: kmacro definition resumes
>>
>> Actual: error
>>
>> Debugger entered--Lisp error: (wrong-type-argument arrayp nil)
>>   start-kbd-macro(t t)
> Thank you for the report.
> I)
>   Isn't it this behavior expected?
>   C-g ends `start-kbd-macro' before any macro has
>   being saved; i.e., `last-kbd-macro' is nil, so we cannot append to it.
>
>   You must have a saved macro to append:
>   emacs -Q
>   F3
>   (insert "a") RET
>   F4 ; save it in `last-kbd-macro'
>
>   C-u C-u F3 ; Apped to it.
>   (insert "b") RET
>   F4 ; Save it.
>
>   F4 ; This insert "ab" in the current buffer.
>
> 2)
>   Expected or not, i think `kmacro-start-macro' might throw an error
>   when the user wants to append and `start-kbd-macro' is nil.
>
> --8<-----------------------------cut here---------------start------------->8---
> commit 9c86eed0b015950a4ae06243c5807d9b864eb69f
> Author: Tino Calancha <[hidden email]>
> Date:   Tue Aug 8 14:14:55 2017 +0900
>
>     Append kbd macro only if last-kbd-macro is non-nil
>
>     * lisp/kmacro.el (kmacro-start-macro): Append only if
>     last-kbd-macro is non-nil (Bug#28008).
>
> diff --git a/lisp/kmacro.el b/lisp/kmacro.el
> index 2db8061fa4..8eff7e5c2e 100644
> --- a/lisp/kmacro.el
> +++ b/lisp/kmacro.el
> @@ -584,7 +584,8 @@ kmacro-start-macro
>               kmacro-last-counter kmacro-counter
>               kmacro-counter-format kmacro-default-counter-format
>               kmacro-counter-format-start kmacro-default-counter-format))
> -
> +      (when (and append (null last-kbd-macro))
> +        (user-error "No kbd macro has been defined"))
>        (start-kbd-macro append
>                        (and append
>                             (if kmacro-execute-before-append
> --8<-----------------------------cut here---------------end--------------->8---
> In GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
>  of 2017-08-08
> Repository revision: c3445aed51944becb3e58f5dace8121c0021f6c7



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#28008: 25.2; Resume kmacro definition errors C-u C-u <F3>

Tino Calancha-2
Allen Li <[hidden email]> writes:

> That does make a kind of sense, but it seems to make the feature
> useless for half the cases where you would want to use it.
>
> I found others with the same use case asking the same question:
> https://emacs.stackexchange.com/questions/3211/how-to-resume-an-incomplete-keyboard-macro-recording
>
> Having any input error cause the macro recording to die makes
> recording macros precarious.  One accidental typo, say C-x instead of
> C-c, and you can't even C-g out of it, without having to start over or
> take a detour through lossage.
>
> I can understand a bare C-g canceling the macro definition, but why
> not have errors save the macro definition?  I can't think of any case
> where I would want an error to not save the macro I have been
> painstakingly defining, but plenty of cases where I would want to
> resume or edit a macro definition after an error.  Heck, there's the
> macro ring if you really wanted the previous macro.

Following are examples for all snippets suggested in this thread and the
stackexchage question:

A. In the stackexchange question, the OP says that after 4. below
   your macro is lost:
   1. f3
   2. hello
   3. C-s
   4. C-g

   ;; but it's not lost: the macro was saved:
   M-: last-kbd-macro RET
   "hello"
   ;; You can edit it:
   5. C-x C-k C-e ; edit as needed
   6. C-c C-c
   7. C-u C-u f3
   8. SPC world f4

   ;; Now you have recorded a macro with inserts
   ;; "hello world" in current buffer.
   F4 ; call the macro.

B. Same applies to the following example:
   1. f3
   2. ab
   3. Alt-x ; Mistake: you don't want this.
   4. C-g
   5. M-: last-kbd-macro RET ; The macro is saved.
      => "ab"

C. AFAICT, if we get an error the macro is saved as well:
   ;; Write a file /tmp/macro.el with content:
   --8<-----------------------------cut here---------------start------------->8---
   (defun test ()
     (interactive)
     (error "Got an error while defininig a kbd macro"))

   (global-set-key (kbd "<f8>") 'test)
   --8<-----------------------------cut here---------------end--------------->8---
   1. emacs -Q /tmp/macro.el
   2. f3 ab f8 ; An error stop kbd macro recording.
   3. M-: last-kbd-macro RET ; Macro was saved.
      => "ab"

D. Endeed, with your recipes the macro is lost:
   D.1. First recipe:
        1. emacs -Q
        2. F3
        3. my namo ; Type some stuff
        4. C-g
        5. C-u C-u F3
        ;; here we haven't saved a macro:
        M-: last-kbd-macro RET
        nil

        ;; An user should use 'DEL' rather than 'C-g' in 4. above:
        ;; (Should we remark this in the manual/docstring?)
        1. emacs -Q
        2. F3
        3. my namo ; Type some stuff: upps a typo! Must read 'my name'
        4. DEL e SPC is SPC John SPC Doe F4
        ;; Now you have recorded a macro with inserts
        ;; "my name is John Doe" in current buffer.
        M-: last-kbd-macro RET
        => [109 121 32 110 97 109 111 backspace 101 32 105 115 ...]

   D.2. Second recipe: This seems quite annoying.
        1. emacs -Q
        2. F3
        3. ab
        4. C-x ; mistake
        5. C-g
        6. M-: last-kbd-macro RET
           => nil

* The main concern is D.2: it looks similar than A, B, but it doesn't save
  the current kbd macro recording.

 ** Patch 1 always save the macro in `last-kbd-macro' after an error or 'C-g'.
    Then, A. B. and D.2 behaves similarly.

 ** Patch 2 adds a new variable `last-aborted-kbd-macro': it saves the partial
    macro there after an error or 'C-g'.  Called `start-kbd-macro' with
    APPEND non-nil offers to append on `last-aborted-kbd-macro'; possible answers
    are 'yes', 'no' or 'del' (i.e., append on `last-aborted-kbd-macro' after delete
    its last character).

    This is not backward compatible; for instance, the snippets A, B above won't be
    saved in `last-kbd-macro' (currently they do).
    It's more tidy; it separates 'good macros', i.e. those ended after
    'F4' or 'C-x )', from 'partial macros', i.e., those ended after an error or 'C-g'.


;;; Patch 1
--8<-----------------------------cut here---------------start------------->8---
commit ce21412d69deeffc55f2c891886351e1384f88cd
Author: Tino Calancha <[hidden email]>
Date:   Fri Aug 11 21:08:48 2017 +0900

    Save aborted kbd macro definitions
   
    While a defining a kbd macro, if we get an error or if the user inputs C-g,
    then save the aborted kbd macro record (Bug#28008).
   
    * lisp/kmacro.el (kmacro-start-macro): Signal an error if APPEND is non-nil
    and last-kbd-macro is nil.
   
    * src/keyboard.c (cmd_error): Increase buffer size for macroerror
    to accommodate new error message.
    If we are defining a kbd macro and we got an error, then save the
    current progress in last-kbd-macro.
    (init_kboard): Initialize last-aborted-kbd-macro.
    (mark_kboards): Mark last-aborted-kbd-macro.
   
    * src/keyboard.h (save_aborted_kbd_macro): Declare this function.
   
    * src/macros.c (save_aborted_kbd_macro): New function.
    (store_kbd_macro_char): Call save_aborted_kbd_macro when user inputs C-g.

diff --git a/lisp/kmacro.el b/lisp/kmacro.el
index 2db8061fa4..8eff7e5c2e 100644
--- a/lisp/kmacro.el
+++ b/lisp/kmacro.el
@@ -584,7 +584,8 @@ kmacro-start-macro
       kmacro-last-counter kmacro-counter
       kmacro-counter-format kmacro-default-counter-format
       kmacro-counter-format-start kmacro-default-counter-format))
-
+      (when (and append (null last-kbd-macro))
+        (user-error "No kbd macro has been defined"))
       (start-kbd-macro append
        (and append
     (if kmacro-execute-before-append
diff --git a/src/keyboard.c b/src/keyboard.c
index 97069a24ac..5111e2c358 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -941,7 +941,8 @@ static Lisp_Object
 cmd_error (Lisp_Object data)
 {
   Lisp_Object old_level, old_length;
-  char macroerror[sizeof "After..kbd macro iterations: "
+  char macroerror[sizeof "Saved aborted kbd macro in \
+`last-kbd-macro' after error: "
   + INT_STRLEN_BOUND (EMACS_INT)];
 
 #ifdef HAVE_WINDOW_SYSTEM
@@ -949,7 +950,13 @@ cmd_error (Lisp_Object data)
     cancel_hourglass ();
 #endif
 
-  if (!NILP (executing_kbd_macro))
+  if (!NILP (KVAR (current_kboard, defining_kbd_macro)))
+    {
+      sprintf (macroerror,
+               "Saved aborted kbd macro in `last-kbd-macro' after error: ");
+      save_aborted_kbd_macro (false);
+    }
+  else if (!NILP (executing_kbd_macro))
     {
       if (executing_kbd_macro_iterations == 1)
  sprintf (macroerror, "After 1 kbd macro iteration: ");
diff --git a/src/keyboard.h b/src/keyboard.h
index 2219c01135..676ccd83cc 100644
--- a/src/keyboard.h
+++ b/src/keyboard.h
@@ -463,6 +463,8 @@ extern bool lucid_event_type_list_p (Lisp_Object);
 extern void kbd_buffer_store_event (struct input_event *);
 extern void kbd_buffer_store_buffered_event (union buffered_input_event *,
      struct input_event *);
+extern Lisp_Object save_aborted_kbd_macro (bool);
+
 INLINE void
 kbd_buffer_store_event_hold (struct input_event *event,
      struct input_event *hold_quit)
diff --git a/src/macros.c b/src/macros.c
index f0ffda3f44..8e45b5fce7 100644
--- a/src/macros.c
+++ b/src/macros.c
@@ -39,15 +39,50 @@ EMACS_INT executing_kbd_macro_iterations;
 
 Lisp_Object executing_kbd_macro;
 
+/* Save the aborted macro.
+   Called if an error happens, or if the user inputs C-g,
+   while defining a kbd macro.  */
+
+Lisp_Object
+save_aborted_kbd_macro (bool msg)
+{
+  struct kboard *kb = current_kboard;
+  /* Must contain something; otherwise don't save it. */
+  if (kb->kbd_macro_end != kb->kbd_macro_buffer)
+    {
+      end_kbd_macro ();
+      if (msg)
+        {
+          message1 ("Saved aborted kbd macro in `last-kbd-macro'");
+          /* Set inhibit_quit to until sleep_for ends */
+          Vinhibit_quit = Qt;
+          Fsleep_for (make_number (1), Qnil);
+          Vinhibit_quit = Qnil;
+        }
+    }
+
+  return Qnil;
+}
+
+
 DEFUN ("start-kbd-macro", Fstart_kbd_macro, Sstart_kbd_macro, 1, 2, "P",
        doc: /* Record subsequent keyboard input, defining a keyboard macro.
 The commands are recorded even as they are executed.
 Use \\[end-kbd-macro] to finish recording and make the macro available.
 Use \\[name-last-kbd-macro] to give it a permanent name.
-Non-nil arg (prefix arg) means append to last macro defined;
-this begins by re-executing that macro as if you typed it again.
-If optional second arg, NO-EXEC, is non-nil, do not re-execute last
-macro before appending to it.  */)
+A call to \\[keyboard-quit] aborts the record and sets `last-aborted-kbd-macro'.
+
+If optional arg APPEND is non-nil, then append to last macro defined;
+ this begins by re-executing that macro as if you typed it again.  Called
+ interactively with a prefix set APPEND non-nil.
+If optional second arg NO-EXEC is non-nil, do not re-execute last
+ macro before appending to it.  Called interactively with 2 prefices
+ set NO-EXEC non-nil.
+If `last-aborted-kbd-macro' is non-nil, then ask if we must
+ append on it; there are 3 possible options:
+ yes: append on `last-aborted-kbd-macro'.
+ no: append on `last-kbd-macro', if non-nil.
+ del: append on `last-aborted-kbd-macro' after skip its last character.*/)
   (Lisp_Object append, Lisp_Object no_exec)
 {
   if (!NILP (KVAR (current_kboard, defining_kbd_macro)))
@@ -182,6 +217,10 @@ store_kbd_macro_char (Lisp_Object c)
 
   if (!NILP (KVAR (kb, defining_kbd_macro)))
     {
+      /* We received a Quit: save the current kboard in Vlast_kbd_macro */
+      if (XFASTINT (c) == quit_char)
+        save_aborted_kbd_macro (true);
+
       if (kb->kbd_macro_ptr - kb->kbd_macro_buffer == kb->kbd_macro_bufsize)
  {
   ptrdiff_t ptr_offset = kb->kbd_macro_ptr - kb->kbd_macro_buffer;

--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2017-08-09
Repository revision: e3ed43f4ac667d39fffcc48cfbe97b074f9aa5c7


;;; Patch 2
--8<-----------------------------cut here---------------start------------->8---
commit da8cf2a563eb25609562fe12c1dd98fc45befdf1
Author: Tino Calancha <[hidden email]>
Date:   Fri Aug 11 21:09:31 2017 +0900

    Save aborted kbd macro definitions
   
    Add a new variable 'last-aborted-kbd-macro'; if we get an error, or if
    the user inputs C-g, then save the aborted kbd macro record on this
    variable.
    When called 'start-kbd-macro' with APPEND non-nil,  offer to append on
    'last-aborted-kbd-macro', if this variable is non-nil (Bug#28008).
   
    * lisp/kmacro.el (kmacro-start-macro): Signal an error if APPEND is non-nil
    and both, last-kbd-macro and last-aborted-kbd-macro are nil.
   
    * src/keyboard.c (cmd_error): Increase buffer size for macroerror
    to accommodate new error message.
    If we are defining a kbd macro, then save the current progress in
    last-aborted-kbd-macro.
    (init_kboard): Initialize last-aborted-kbd-macro.
    (mark_kboards): Mark last-aborted-kbd-macro.
   
    * src/keyboard.h (struct kboard): Add new member Vlast_aborted_kbd_macro_.
    (kset_last_aborted_kbd_macro): Add setter.
    (save_aborted_kbd_macro): Declare this function.
   
    * src/macros.c (last-aborted-kbd-macro): New variable.
    (save_aborted_kbd_macro): New function.
    (start-kbd-macro): Offer to append on last-aborted-kbd-macro, if non-nil.
    (store_kbd_macro_char): Call save_aborted_kbd_macro when user inputs C-g.

diff --git a/lisp/kmacro.el b/lisp/kmacro.el
index 2db8061fa4..2e79269fd1 100644
--- a/lisp/kmacro.el
+++ b/lisp/kmacro.el
@@ -584,7 +584,8 @@ kmacro-start-macro
       kmacro-last-counter kmacro-counter
       kmacro-counter-format kmacro-default-counter-format
       kmacro-counter-format-start kmacro-default-counter-format))
-
+      (when (and append (null (or last-kbd-macro last-aborted-kbd-macro)))
+        (user-error "No kbd macro has been defined"))
       (start-kbd-macro append
        (and append
     (if kmacro-execute-before-append
diff --git a/src/keyboard.c b/src/keyboard.c
index 97069a24ac..79bad93a5b 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -941,7 +941,8 @@ static Lisp_Object
 cmd_error (Lisp_Object data)
 {
   Lisp_Object old_level, old_length;
-  char macroerror[sizeof "After..kbd macro iterations: "
+  char macroerror[sizeof "Saved aborted kbd macro in \
+`last-aborted-kbd-macro' after error: "
   + INT_STRLEN_BOUND (EMACS_INT)];
 
 #ifdef HAVE_WINDOW_SYSTEM
@@ -949,7 +950,13 @@ cmd_error (Lisp_Object data)
     cancel_hourglass ();
 #endif
 
-  if (!NILP (executing_kbd_macro))
+  if (!NILP (KVAR (current_kboard, defining_kbd_macro)))
+    {
+      sprintf (macroerror,
+               "Saved aborted kbd macro in `last-aborted-kbd-macro' after error: ");
+      save_aborted_kbd_macro (false);
+    }
+  else if (!NILP (executing_kbd_macro))
     {
       if (executing_kbd_macro_iterations == 1)
  sprintf (macroerror, "After 1 kbd macro iteration: ");
@@ -10864,6 +10871,7 @@ init_kboard (KBOARD *kb, Lisp_Object type)
   kb->kbd_macro_bufsize = 0;
   kset_defining_kbd_macro (kb, Qnil);
   kset_last_kbd_macro (kb, Qnil);
+  kset_last_aborted_kbd_macro (kb, Qnil);
   kb->reference_count = 0;
   kset_system_key_alist (kb, Qnil);
   kset_system_key_syms (kb, Qnil);
@@ -11977,6 +11985,7 @@ mark_kboards (void)
       mark_object (KVAR (kb, kbd_queue));
       mark_object (KVAR (kb, defining_kbd_macro));
       mark_object (KVAR (kb, Vlast_kbd_macro));
+      mark_object (KVAR (kb, Vlast_aborted_kbd_macro));
       mark_object (KVAR (kb, Vsystem_key_alist));
       mark_object (KVAR (kb, system_key_syms));
       mark_object (KVAR (kb, Vwindow_system));
diff --git a/src/keyboard.h b/src/keyboard.h
index 2219c01135..24868f6dc1 100644
--- a/src/keyboard.h
+++ b/src/keyboard.h
@@ -133,6 +133,9 @@ struct kboard
     /* Last anonymous kbd macro defined.  */
     Lisp_Object Vlast_kbd_macro_;
 
+    /* Last interrupted kbd macro.  */
+    Lisp_Object Vlast_aborted_kbd_macro_;
+
     /* Alist of system-specific X windows key symbols.  */
     Lisp_Object Vsystem_key_alist_;
 
@@ -207,6 +210,11 @@ kset_last_kbd_macro (struct kboard *kb, Lisp_Object val)
   kb->Vlast_kbd_macro_ = val;
 }
 INLINE void
+kset_last_aborted_kbd_macro (struct kboard *kb, Lisp_Object val)
+{
+  kb->Vlast_aborted_kbd_macro_ = val;
+}
+INLINE void
 kset_prefix_arg (struct kboard *kb, Lisp_Object val)
 {
   kb->Vprefix_arg_ = val;
@@ -463,6 +471,8 @@ extern bool lucid_event_type_list_p (Lisp_Object);
 extern void kbd_buffer_store_event (struct input_event *);
 extern void kbd_buffer_store_buffered_event (union buffered_input_event *,
      struct input_event *);
+extern Lisp_Object save_aborted_kbd_macro (bool);
+
 INLINE void
 kbd_buffer_store_event_hold (struct input_event *event,
      struct input_event *hold_quit)
diff --git a/src/macros.c b/src/macros.c
index f0ffda3f44..6c33d420c7 100644
--- a/src/macros.c
+++ b/src/macros.c
@@ -39,15 +39,53 @@ EMACS_INT executing_kbd_macro_iterations;
 
 Lisp_Object executing_kbd_macro;
 
+/* Save the aborted macro.
+   Called if an error happens, or if the user inputs C-g,
+   while defining a kbd macro.  */
+
+Lisp_Object
+save_aborted_kbd_macro (bool msg)
+{
+  struct kboard *kb = current_kboard;
+  Lisp_Object last_macro = KVAR (kb, Vlast_kbd_macro);
+  /* Must contain something; otherwise don't save it. */
+  if (kb->kbd_macro_end != kb->kbd_macro_buffer)
+    {
+      end_kbd_macro ();
+      kset_last_aborted_kbd_macro (kb, KVAR (kb, Vlast_kbd_macro));
+      kset_last_kbd_macro (kb, last_macro);
+      if (msg)
+        {
+          message1 ("Saved aborted kbd macro in `last-aborted-kbd-macro'");
+          /* Set inhibit_quit to until sleep_for ends */
+          Vinhibit_quit = Qt;
+          Fsleep_for (make_number (1), Qnil);
+          Vinhibit_quit = Qnil;
+        }
+    }
+
+  return Qnil;
+}
+
+
 DEFUN ("start-kbd-macro", Fstart_kbd_macro, Sstart_kbd_macro, 1, 2, "P",
        doc: /* Record subsequent keyboard input, defining a keyboard macro.
 The commands are recorded even as they are executed.
 Use \\[end-kbd-macro] to finish recording and make the macro available.
 Use \\[name-last-kbd-macro] to give it a permanent name.
-Non-nil arg (prefix arg) means append to last macro defined;
-this begins by re-executing that macro as if you typed it again.
-If optional second arg, NO-EXEC, is non-nil, do not re-execute last
-macro before appending to it.  */)
+A call to \\[keyboard-quit] aborts the record and sets `last-aborted-kbd-macro'.
+
+If optional arg APPEND is non-nil, then append to last macro defined;
+ this begins by re-executing that macro as if you typed it again.  Called
+ interactively with a prefix set APPEND non-nil.
+If optional second arg NO-EXEC is non-nil, do not re-execute last
+ macro before appending to it.  Called interactively with 2 prefices
+ set NO-EXEC non-nil.
+If `last-aborted-kbd-macro' is non-nil, then ask if we must
+ append on it; there are 3 possible options:
+ yes: append on `last-aborted-kbd-macro'.
+ no: append on `last-kbd-macro', if non-nil.
+ del: append on `last-aborted-kbd-macro' after skip its last character.*/)
   (Lisp_Object append, Lisp_Object no_exec)
 {
   if (!NILP (KVAR (current_kboard, defining_kbd_macro)))
@@ -79,9 +117,59 @@ macro before appending to it.  */)
       int incr = 30;
       ptrdiff_t i, len;
       bool cvt;
+      Lisp_Object last_macro = KVAR (current_kboard, Vlast_kbd_macro);
+      /* If true, then drop last character from the aborted macro. */
+      bool del_last;
+      /* If there is a previous aborted macro, offer to use it. */
+      if (!NILP (KVAR (current_kboard, Vlast_aborted_kbd_macro)))
+        {
+          Lisp_Object ans;
+          bool loop_end = 1;
+          AUTO_STRING (prompt, "Use aborted macro? [yes, no, del] ");
+          while (loop_end)
+            {
+              ans = Fdowncase (Fread_from_minibuffer (prompt, Qnil, Qnil, Qnil,
+                                                      Qnil, Qnil, Qnil));
+              if (SCHARS (ans) == 3 && (!strcmp (SSDATA (ans), "yes")
+                                        || !strcmp (SSDATA (ans), "del")))
+                {
+                  last_macro = KVAR (current_kboard, Vlast_aborted_kbd_macro);
+                  /* We cannot execute an aborted macro */
+                  if (NILP (no_exec))
+                    {
+                      message1 ("We cannot execute an aborted macro");
+                      no_exec = Qt;
+                    }
+                  if (!strcmp (SSDATA (ans), "del"))
+                    {
+                      del_last = 1;
+                      /* current_kboard->kbd_macro_end--; */
+                      /* current_kboard->kbd_macro_ptr--; */
+                    }
+                  loop_end = 0;
+                }
+              else if (SCHARS (ans) == 2 && !strcmp (SSDATA (ans), "no"))
+                {
+                  if (!NILP (KVAR (current_kboard, Vlast_kbd_macro)))
+                    last_macro = KVAR (current_kboard, Vlast_kbd_macro);
+                  else
+                    error ("No kbd macro has been defined");
+                  loop_end = 0;
+                }
+              else
+                {
+                  Fding (Qnil);
+                  Fdiscard_input ();
+                  message1 ("Please answer yes or no or del");
+                  Fsleep_for (make_number (2), Qnil);
+                }
+            }
+          kset_last_aborted_kbd_macro (current_kboard, Qnil);
+        }
 
       /* Check the type of last-kbd-macro in case Lisp code changed it.  */
-      len = CHECK_VECTOR_OR_STRING (KVAR (current_kboard, Vlast_kbd_macro));
+      len = CHECK_VECTOR_OR_STRING (last_macro);
+      if (del_last) len--;
 
       /* Copy last-kbd-macro into the buffer, in case the Lisp code
  has put another macro there.  */
@@ -93,11 +181,11 @@ macro before appending to it.  */)
    sizeof *current_kboard->kbd_macro_buffer);
 
       /* Must convert meta modifier when copying string to vector.  */
-      cvt = STRINGP (KVAR (current_kboard, Vlast_kbd_macro));
+      cvt = STRINGP (last_macro);
       for (i = 0; i < len; i++)
  {
   Lisp_Object c;
-  c = Faref (KVAR (current_kboard, Vlast_kbd_macro), make_number (i));
+  c = Faref (last_macro, make_number (i));
   if (cvt && NATNUMP (c) && (XFASTINT (c) & 0x80))
     XSETFASTINT (c, CHAR_META | (XFASTINT (c) & ~0x80));
   current_kboard->kbd_macro_buffer[i] = c;
@@ -109,8 +197,7 @@ macro before appending to it.  */)
       /* Re-execute the macro we are appending to,
  for consistency of behavior.  */
       if (NILP (no_exec))
- Fexecute_kbd_macro (KVAR (current_kboard, Vlast_kbd_macro),
-    make_number (1), Qnil);
+ Fexecute_kbd_macro (last_macro, make_number (1), Qnil);
 
       message1 ("Appending to kbd macro...");
     }
@@ -182,6 +269,10 @@ store_kbd_macro_char (Lisp_Object c)
 
   if (!NILP (KVAR (kb, defining_kbd_macro)))
     {
+      /* We received a Quit: save the current kboard in Vlast_aborted_kbd_macro */
+      if (XFASTINT (c) == quit_char)
+        save_aborted_kbd_macro (true);
+
       if (kb->kbd_macro_ptr - kb->kbd_macro_buffer == kb->kbd_macro_bufsize)
  {
   ptrdiff_t ptr_offset = kb->kbd_macro_ptr - kb->kbd_macro_buffer;
@@ -374,4 +465,7 @@ This is nil when not executing a keyboard macro.  */);
 
   DEFVAR_KBOARD ("last-kbd-macro", Vlast_kbd_macro,
  doc: /* Last kbd macro defined, as a string or vector; nil if none defined.  */);
+
+  DEFVAR_KBOARD ("last-aborted-kbd-macro", Vlast_aborted_kbd_macro,
+ doc: /* Last aborted kbd macro, as a string or vector; nil if none exist.  */);
 }

--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2017-08-09
Repository revision: e3ed43f4ac667d39fffcc48cfbe97b074f9aa5c7



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#28008: 25.2; Resume kmacro definition errors C-u C-u <F3>

Eli Zaretskii
> From: Tino Calancha <[hidden email]>
> Cc: Eli Zaretskii <[hidden email]>,  [hidden email], [hidden email]
> Date: Fri, 11 Aug 2017 21:41:58 +0900
>
>  ** Patch 1 always save the macro in `last-kbd-macro' after an error or 'C-g'.
>     Then, A. B. and D.2 behaves similarly.
>
>  ** Patch 2 adds a new variable `last-aborted-kbd-macro': it saves the partial
>     macro there after an error or 'C-g'.  Called `start-kbd-macro' with
>     APPEND non-nil offers to append on `last-aborted-kbd-macro'; possible answers
>     are 'yes', 'no' or 'del' (i.e., append on `last-aborted-kbd-macro' after delete
>     its last character).
>
>     This is not backward compatible; for instance, the snippets A, B above won't be
>     saved in `last-kbd-macro' (currently they do).
>     It's more tidy; it separates 'good macros', i.e. those ended after
>     'F4' or 'C-x )', from 'partial macros', i.e., those ended after an error or 'C-g'.

All these low-level changes just to support an obscure use case?  Is
really worth the risk to break macros to cater to that?

Thanks.



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#28008: 25.2; Resume kmacro definition errors C-u C-u <F3>

Tino Calancha-2


On Fri, 11 Aug 2017, Eli Zaretskii wrote:

>>  ** Patch 1 always save the macro in `last-kbd-macro' after an error or 'C-g'.
>>     Then, A. B. and D.2 behaves similarly.
>>
>>  ** Patch 2 adds a new variable `last-aborted-kbd-macro': it saves the partial
>>     macro there after an error or 'C-g'.  Called `start-kbd-macro' with
>>     APPEND non-nil offers to append on `last-aborted-kbd-macro'; possible answers
>>     are 'yes', 'no' or 'del' (i.e., append on `last-aborted-kbd-macro' after delete
>>     its last character).
>>
>>     This is not backward compatible; for instance, the snippets A, B above won't be
>>     saved in `last-kbd-macro' (currently they do).
>>     It's more tidy; it separates 'good macros', i.e. those ended after
>>     'F4' or 'C-x )', from 'partial macros', i.e., those ended after an error or 'C-g'.
>
> All these low-level changes just to support an obscure use case?  Is
> really worth the risk to break macros to cater to that?
That depends of how often someone uses kbd macros.  I rarely use
them, but the people using them frequently might suffer D.2 from time to
time.

Actually, the patch#1 is quite short: i included a docstring fix from
the patch#2 by mistake.
The C code changes in patch#1 are just:
  3 files changed, 41 insertions(+), 2 deletions(-)

Here is patch#1 upated:

--8<-----------------------------cut here---------------start------------->8---
commit fe424d1371ec467b9a257fa75c8c3f734135e6dd
Author: Tino Calancha <[hidden email]>
Date:   Fri Aug 11 22:11:08 2017 +0900

     Save aborted kbd macro definitions

     While a defining a kbd macro, if we get an error or if the user inputs C-g,
     then save the aborted kbd macro record (Bug#28008).

     * lisp/kmacro.el (kmacro-start-macro): Signal an error if APPEND is non-nil
     and last-kbd-macro is nil.

     * src/keyboard.c (cmd_error): Increase buffer size for macroerror
     to accommodate new error message.
     If we are defining a kbd macro and we got an error, then save the
     current progress in last-kbd-macro.
     (init_kboard): Initialize last-aborted-kbd-macro.
     (mark_kboards): Mark last-aborted-kbd-macro.

     * src/keyboard.h (save_aborted_kbd_macro): Declare this function.

     * src/macros.c (save_aborted_kbd_macro): New function.
     (store_kbd_macro_char): Call save_aborted_kbd_macro when user inputs C-g.

diff --git a/lisp/kmacro.el b/lisp/kmacro.el
index 2db8061fa4..8eff7e5c2e 100644
--- a/lisp/kmacro.el
+++ b/lisp/kmacro.el
@@ -584,7 +584,8 @@ kmacro-start-macro
       kmacro-last-counter kmacro-counter
       kmacro-counter-format kmacro-default-counter-format
       kmacro-counter-format-start kmacro-default-counter-format))
-
+      (when (and append (null last-kbd-macro))
+        (user-error "No kbd macro has been defined"))
        (start-kbd-macro append
        (and append
     (if kmacro-execute-before-append
diff --git a/src/keyboard.c b/src/keyboard.c
index 97069a24ac..5111e2c358 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -941,7 +941,8 @@ static Lisp_Object
  cmd_error (Lisp_Object data)
  {
    Lisp_Object old_level, old_length;
-  char macroerror[sizeof "After..kbd macro iterations: "
+  char macroerror[sizeof "Saved aborted kbd macro in \
+`last-kbd-macro' after error: "
   + INT_STRLEN_BOUND (EMACS_INT)];

  #ifdef HAVE_WINDOW_SYSTEM
@@ -949,7 +950,13 @@ cmd_error (Lisp_Object data)
      cancel_hourglass ();
  #endif

-  if (!NILP (executing_kbd_macro))
+  if (!NILP (KVAR (current_kboard, defining_kbd_macro)))
+    {
+      sprintf (macroerror,
+               "Saved aborted kbd macro in `last-kbd-macro' after error: ");
+      save_aborted_kbd_macro (false);
+    }
+  else if (!NILP (executing_kbd_macro))
      {
        if (executing_kbd_macro_iterations == 1)
  sprintf (macroerror, "After 1 kbd macro iteration: ");
diff --git a/src/keyboard.h b/src/keyboard.h
index 2219c01135..676ccd83cc 100644
--- a/src/keyboard.h
+++ b/src/keyboard.h
@@ -463,6 +463,8 @@ extern bool lucid_event_type_list_p (Lisp_Object);
  extern void kbd_buffer_store_event (struct input_event *);
  extern void kbd_buffer_store_buffered_event (union buffered_input_event *,
      struct input_event *);
+extern Lisp_Object save_aborted_kbd_macro (bool);
+
  INLINE void
  kbd_buffer_store_event_hold (struct input_event *event,
      struct input_event *hold_quit)
diff --git a/src/macros.c b/src/macros.c
index f0ffda3f44..1935e4fd2f 100644
--- a/src/macros.c
+++ b/src/macros.c
@@ -39,6 +39,32 @@ EMACS_INT executing_kbd_macro_iterations;

  Lisp_Object executing_kbd_macro;

+/* Save the aborted macro.
+   Called if an error happens, or if the user inputs C-g,
+   while defining a kbd macro.  */
+
+Lisp_Object
+save_aborted_kbd_macro (bool msg)
+{
+  struct kboard *kb = current_kboard;
+  /* Must contain something; otherwise don't save it. */
+  if (kb->kbd_macro_end != kb->kbd_macro_buffer)
+    {
+      end_kbd_macro ();
+      if (msg)
+        {
+          message1 ("Saved aborted kbd macro in `last-kbd-macro'");
+          /* Set inhibit_quit to until sleep_for ends */
+          Vinhibit_quit = Qt;
+          Fsleep_for (make_number (1), Qnil);
+          Vinhibit_quit = Qnil;
+        }
+    }
+
+  return Qnil;
+}
+
+
  DEFUN ("start-kbd-macro", Fstart_kbd_macro, Sstart_kbd_macro, 1, 2, "P",
         doc: /* Record subsequent keyboard input, defining a keyboard macro.
  The commands are recorded even as they are executed.
@@ -182,6 +208,10 @@ store_kbd_macro_char (Lisp_Object c)

    if (!NILP (KVAR (kb, defining_kbd_macro)))
      {
+      /* We received a Quit: save the current kboard in Vlast_kbd_macro */
+      if (XFASTINT (c) == quit_char)
+        save_aborted_kbd_macro (true);
+
        if (kb->kbd_macro_ptr - kb->kbd_macro_buffer == kb->kbd_macro_bufsize)
  {
   ptrdiff_t ptr_offset = kb->kbd_macro_ptr - kb->kbd_macro_buffer;
--8<-----------------------------cut here---------------end--------------->8---



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#28008: 25.2; Resume kmacro definition errors C-u C-u <F3>

Tino Calancha-2
Tino Calancha <[hidden email]> writes:

> On Fri, 11 Aug 2017, Eli Zaretskii wrote:
>
>>>  ** Patch 1 always save the macro in `last-kbd-macro' after an error or 'C-g'.
>>>     Then, A. B. and D.2 behaves similarly.
> Actually, the patch#1 is quite short: i included a docstring fix from
> the patch#2 by mistake.
The commit message of patch#1 got too verbose.  It shouldn't mention
about 'last-aborted-kbd-macro' (that variable belongs to patch#2):
Following is a revisede commit message for patch#1:
--8<-----------------------------cut here---------------start------------->8---
Save aborted kbd macro definitions

While a defining a kbd macro, if we get an error or if the user inputs C-g,
then save the aborted kbd macro record (Bug#28008).

* lisp/kmacro.el (kmacro-start-macro): Signal an error if APPEND is non-nil
and last-kbd-macro is nil.

* src/keyboard.c (cmd_error): Increase buffer size for macroerror
to accommodate new error message.
If we are defining a kbd macro and we got an error, then save the
current progress in last-kbd-macro.

* src/macros.c (save_aborted_kbd_macro): New function.
(store_kbd_macro_char): Call save_aborted_kbd_macro when user inputs C-g.
--8<-----------------------------cut here---------------end--------------->8---



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#28008: 25.2; Resume kmacro definition errors C-u C-u <F3>

Allen Li
On Fri, Aug 11, 2017 at 5:41 AM, Tino Calancha <[hidden email]> wrote:

>  ** Patch 1 always save the macro in `last-kbd-macro' after an error or 'C-g'.
>     Then, A. B. and D.2 behaves similarly.
>
>  ** Patch 2 adds a new variable `last-aborted-kbd-macro': it saves the partial
>     macro there after an error or 'C-g'.  Called `start-kbd-macro' with
>     APPEND non-nil offers to append on `last-aborted-kbd-macro'; possible answers
>     are 'yes', 'no' or 'del' (i.e., append on `last-aborted-kbd-macro' after delete
>     its last character).
>
>     This is not backward compatible; for instance, the snippets A, B above won't be
>     saved in `last-kbd-macro' (currently they do).
>     It's more tidy; it separates 'good macros', i.e. those ended after
>     'F4' or 'C-x )', from 'partial macros', i.e., those ended after an error or 'C-g'.

I'm not the best qualified to comment on the patches themselves, but 2
sounds like the best solution except that it breaks backward
compatibility. However, I hypothesize that no one is relying on the
old behavior (that non-quit errors stop the macro recording and yet
save it).

On Fri, Aug 11, 2017 at 6:00 AM, Eli Zaretskii <[hidden email]> wrote:
> All these low-level changes just to support an obscure use case?  Is
> really worth the risk to break macros to cater to that?

I suspect that this bug is one of the reasons that this is an obscure
use case, i.e. that people don't use kmacros as often as they would
like to.  Humans make mistakes, and when they make mistakes that
translates to either an error or a C-g stopping the kmacro definition,
which makes it impractical to use kmacros in the current form.  If it
were easy to recover from an error during kmacro definition, I know I
would use them more.



Loading...