bug#21329: 25.0.50; Flyspell minor mode produces weird effects on keyboard macros

bug#21329: 25.0.50; Flyspell minor mode produces weird effects on keyboard macros

[Mark Karpov]

❧

This is really strange. Here is how to reproduce it in development
version of Emacs:

1. Start Emacs with ‘-Q’ flag.

2. Enable ‘flyspell-mode’ (M-x flyspell-mode RET).

3. Start recording keyboard macro with F3.

4. Type (pick one of these, they all produce different effects):

   * abc
   * prop_

5. Finish recording of the keyboard macro with F4.

6. Hit F4 again to run the macro.

If you chose “abc”, you will get:

abcabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbcca

And in ‘*Messages*’:

For information about GNU Emacs and the GNU system, type C-h C-a.
completing-read-default: Command attempted to use minibuffer while in minibuffer
Quit
Starting new Ispell process /usr/bin/aspell with default dictionary...
Defining kbd macro...
Keyboard macro defined
After 0 kbd macro iterations: run-hook-with-args: Variable binding depth exceeds max-specpdl-size

If you rather preferred “prop_”:

prop_prroopp__

Well. Some letters (all expect for the first one) are duplicated…



In GNU Emacs 25.0.50.18 (x86_64-unknown-linux-gnu, GTK+ Version 3.16.6)
 of 2015-08-19 on arch
Repository revision: 62661fe759dfc826f31b032e49f0f15b40a207d5
Windowing system distributor `The X.Org Foundation', version 11.0.11702000
Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF GSETTINGS
NOTIFY ACL GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: iε

Minor modes in effect:
  rainbow-delimiters-mode: t
  hl-todo-mode: t
  flycheck-mode: t
  flyspell-mode: t
  whitespace-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  show-paren-mode: t
  minibuffer-electric-default-mode: t
  ido-vertical-mode: t
  ido-ubiquitous-mode: t
  ido-everywhere: t
  highlight-line-mode: t
  global-auto-revert-mode: t
  global-aggressive-indent-mode: t
  aggressive-indent-mode: t
  display-time-mode: t
  delete-selection-mode: t
  ace-popup-menu-mode: t
  smartparens-global-mode: t
  smartparens-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  auto-fill-function: do-auto-fill
  transient-mark-mode: t
  auto-fill-mode: 1

Recent messages:
ad-handle-definition: `ido-completing-read' got redefined
[yas] Loading compiled snippets from /home/mark/.emacs.d/elpa/common-lisp-snippets-20150603.944/snippets/lisp-mode [2 times]
[yas] Prepared just-in-time loading for /home/mark/.emacs.d/snippets
[yas] Prepared just-in-time loading for /home/mark/.emacs.d/elpa/common-lisp-snippets-20150603.944/snippets
[yas] Reloaded everything (snippets will load just-in-time)....
Starting new Ispell process /usr/bin/aspell with en dictionary...
[yas] Loading for `emacs-lisp-mode', just-in-time: (lambda nil (yas--load-directory-1 (quote /home/mark/.emacs.d/snippets/emacs-lisp-mode) (quote emacs-lisp-mode)))!
[yas] Loading compiled snippets from /home/mark/.emacs.d/snippets/emacs-lisp-mode
Sunday, 23 August 2015

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message dired format-spec
rfc822 mml mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util help-fns mail-prsvr mail-utils rainbow-delimiters hl-todo
flycheck-haskell haskell-cabal haskell-utils haskell-customize flycheck
find-func rx flyspell ispell whitespace mk-visual solarized-dark-theme
solarized mk-texinfo mk-tex mk-python mk-prolog mk-org mk-markdown
mk-man mk-magit mk-lisp mk-ibuffer mk-haskell mk-gnus mk-eshell mk-erc
mk-elisp mk-dired mk-diff mk-clojure kill-or-bury-alive mk-calendar mk-c
mk-bookmark mk-minor-modes common-lisp-snippets yasnippet cl
whole-line-or-region paren minibuf-eldef ido-vertical-mode
ido-ubiquitous ido-completing-read+ cl-macs gv ido highlight-line
autorevert filenotify cl-seq aggressive-indent easy-mmode time delsel
ace-popup-menu smartparens-config smartparens byte-opt thingatpt
mk-global edmacro kmacro ace-link avy ring misc server mk-utils subr-x f
dash s ucs-normalize finder-inf tex-site fix-word advice slime-autoloads
info bytecomp byte-compile cl-extra help-mode seq cl-loaddefs pcase
cl-lib cconv package easymenu epg-config time-date mule-util tooltip
eldoc electric uniquify ediff-hook vc-hooks lisp-float-type mwheel x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list newcomment elisp-mode lisp-mode prog-mode register page
menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core frame cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese charscript case-table epa-hook jka-cmpr-hook help
simple abbrev minibuffer cl-preloaded nadvice loaddefs button faces
cus-face macroexp files text-properties overlay sha1 md5 base64 format
env code-pages mule custom widget hashtable-print-readable backquote
dbusbind inotify dynamic-setting system-font-setting font-render-setting
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 350582 106916)
 (symbols 48 31571 7)
 (miscs 40 66 212)
 (strings 32 54146 56214)
 (string-bytes 1 1538038)
 (vectors 16 45028)
 (vector-slots 8 781772 45990)
 (floats 8 283 405)
 (intervals 56 248 35)
 (buffers 976 12)
 (heap 1024 60596 4785))

bug#21329: 25.0.50; Flyspell minor mode produces weird effects on keyboard macros

[Eli Zaretskii]

> From: Mark Karpov <markkarpov@openmailbox.org>
> Date: Sun, 23 Aug 2015 17:57:44 +0600
> 
> This is really strange. Here is how to reproduce it in development
> version of Emacs:
> 
> 1. Start Emacs with ‘-Q’ flag.
> 
> 2. Enable ‘flyspell-mode’ (M-x flyspell-mode RET).
> 
> 3. Start recording keyboard macro with F3.
> 
> 4. Type (pick one of these, they all produce different effects):
> 
>    * abc
>    * prop_
> 
> 5. Finish recording of the keyboard macro with F4.
> 
> 6. Hit F4 again to run the macro.
> 
> If you chose “abc”, you will get:
> 
> abcabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbccabbcca
> 
> And in ‘*Messages*’:
> 
> For information about GNU Emacs and the GNU system, type C-h C-a.
> completing-read-default: Command attempted to use minibuffer while in minibuffer
> Quit

This is a very old problem, I see it in Emacs 22.

Here's a simple recipe to demonstrate what happens:

  emacs -Q

  Evaluate in *scratch*:

    (defun my-hook ()
      ""
      (sit-for 1))
    (add-hook 'post-command-hook 'my-hook t t)

  Now type "F3 abc F4 F4", and you will get the same problem as in the
  original report.  Type "C-x C-k n foo RET" followed by "M-x
  insert-kbd-macro RET", and you will see that the macro is defined as
  a vector that includes F4 at its end, which is the cause of the
  unexpected recursive invocation of the macro by F4.

The problem is that 'sit-for' calls 'read-event', and then "undoes"
that by pushing the events it read onto unread-command-events.  This
evidently messes up keyboard macro recording.  (It also runs afoul of
the recent changes in keyboard.c related to recording events, which
causes the characters to be duplicated in the macro.)

The following band-aid fixes the problem, but I will wait for The
Powers That Be to comment on this before pushing.  Or maybe we should
introduce an "unrecord-key" facility for use by code such as we have
in sit-for?

--- lisp/subr.el~	2015-08-05 12:26:16.000000000 +0300
+++ lisp/subr.el	2015-08-24 13:53:09.858014100 +0300
@@ -2276,7 +2276,8 @@
     t)
    ((input-pending-p t)
     nil)
-   ((<= seconds 0)
+   ((or (<= seconds 0)
+        defining-kbd-macro)
     (or nodisp (redisplay)))
    (t
     (or nodisp (redisplay))

bug#21329: 25.0.50; Flyspell minor mode produces weird effects on keyboard macros

[Stefan Monnier]

> The problem is that 'sit-for' calls 'read-event', and then "undoes"
> that by pushing the events it read onto unread-command-events.

Indeed, that's a problem.  The right fix would be to change the code so
as not to consume any event in the first place.

> Or maybe we should introduce an "unrecord-key" facility for use by
> code such as we have in sit-for?

We could try, but I don't believe we can provide an "unrecord-key"
that's actually reliable.  AFAIK the act of taking an event out of the
queue has too many effects to have a hope of properly reverting
them all.


        Stefan

bug#21329: 25.0.50; Flyspell minor mode produces weird effects on keyboard macros

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Mark Karpov <markkarpov@openmailbox.org>,  21329@debbugs.gnu.org
> Date: Tue, 25 Aug 2015 09:15:58 -0400
> 
> > The problem is that 'sit-for' calls 'read-event', and then "undoes"
> > that by pushing the events it read onto unread-command-events.
> 
> Indeed, that's a problem.  The right fix would be to change the code so
> as not to consume any event in the first place.

Any more specific advice?  I'm afraid I'm otherwise not up to the job.

bug#21329: 25.0.50; Flyspell minor mode produces weird effects on keyboard macros

[Eli Zaretskii]

> Date: Tue, 25 Aug 2015 18:26:53 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: markkarpov@openmailbox.org, 21329@debbugs.gnu.org
> 
> > The right fix would be to change the code so as not to consume any
> > event in the first place.
> 
> Any more specific advice?  I'm afraid I'm otherwise not up to the job.

Would it work to loop until input-pending-p returns non-nil?

bug#21329: 25.0.50; Flyspell minor mode produces weird effects on keyboard macros

[Stefan Monnier]

> Would it work to loop until input-pending-p returns non-nil?

Maybe that can be made to work, but it's likely to introduce
other problems.

I was thinking more about refactoring read-event so as to provide a new
`wait-for-event` which stops just before we actually consume the event.

Of course, since I'm sure you've already seen some part of read-event,
you're probably already (and sanely) running away screaming,


        Stefan

bug#21329: 25.0.50; Flyspell minor mode produces weird effects on keyboard macros

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: markkarpov@openmailbox.org,  21329@debbugs.gnu.org
> Date: Tue, 25 Aug 2015 18:23:42 -0400
> 
> > Would it work to loop until input-pending-p returns non-nil?
> 
> Maybe that can be made to work, but it's likely to introduce
> other problems.

Which ones, for example?

> I was thinking more about refactoring read-event so as to provide a new
> `wait-for-event` which stops just before we actually consume the event.

How will it be different from what input-pending-p already does?

> Of course, since I'm sure you've already seen some part of read-event,
> you're probably already (and sanely) running away screaming,

Yep.

bug#21329: 25.0.50; Flyspell minor mode produces weird effects on keyboard macros

[Stefan Monnier]

>> > Would it work to loop until input-pending-p returns non-nil?
>> Maybe that can be made to work, but it's likely to introduce
>> other problems.
> Which ones, for example?

Not sure.  I just know that we've already tried various different
alternatives for this code.  All the problems have been "corner cases",
IIRC, and many of them were nasty enough to make us rewrite the
function completely.

One possible problem is that input-pending-p might return t in cases where
the pending input is "not significant", in the sense that read-event
won't return it (maybe because it will consume it internally as in the
case of event bounds to special-event-map, IIRC).

>> I was thinking more about refactoring read-event so as to provide a new
>> `wait-for-event` which stops just before we actually consume the event.
> How will it be different from what input-pending-p already does?

It will be different in that it will wait until an event arrives.
But yes, maybe the implementation of the two can be shared.


        Stefan


PS: I suggest you try and install your suggested patch, in the
mean time.  I don't understand enough of its impact to judge its
quality, but it's worth a try.  Just make sure you add a good comment in
the code explaining the problem this is trying to fix.

bug#21329: 25.0.50; Flyspell minor mode produces weird effects on keyboard macros

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: markkarpov@openmailbox.org,  21329@debbugs.gnu.org
> Date: Thu, 27 Aug 2015 21:33:16 -0400
> 
> One possible problem is that input-pending-p might return t in cases where
> the pending input is "not significant", in the sense that read-event
> won't return it (maybe because it will consume it internally as in the
> case of event bounds to special-event-map, IIRC).

I thought about ignoring out these kinds of input in the loop that
waits.

> PS: I suggest you try and install your suggested patch, in the
> mean time.  I don't understand enough of its impact to judge its
> quality, but it's worth a try.  Just make sure you add a good comment in
> the code explaining the problem this is trying to fix.

Done.

I'm therefore closing this bug.

bug#21329: 25.0.50; Flyspell minor mode produces weird effects on keyboard macros

[Stefan Monnier]

>> One possible problem is that input-pending-p might return t in cases where
>> the pending input is "not significant", in the sense that read-event
>> won't return it (maybe because it will consume it internally as in the
>> case of event bounds to special-event-map, IIRC).
> I thought about ignoring out these kinds of input in the loop that
> waits.

Then you have another side-effect, which is to delay the processing of
those events.  So instead of ignoring them it should process them.

That's why I think we'd want to "simply" extract all that read-event
does upto (not not including) the actual consumption (and return) of
the event.

>> PS: I suggest you try and install your suggested patch, in the
>> mean time.  I don't understand enough of its impact to judge its
>> quality, but it's worth a try.  Just make sure you add a good comment in
>> the code explaining the problem this is trying to fix.
> Done.
> I'm therefore closing this bug.

Thanks,


        Stefan

bug#21329: 25.0.50; Flyspell minor mode produces weird effects on keyboard macros

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: markkarpov@openmailbox.org,  21329-done@debbugs.gnu.org
> Date: Sat, 29 Aug 2015 11:31:35 -0400
> 
> >> One possible problem is that input-pending-p might return t in cases where
> >> the pending input is "not significant", in the sense that read-event
> >> won't return it (maybe because it will consume it internally as in the
> >> case of event bounds to special-event-map, IIRC).
> > I thought about ignoring out these kinds of input in the loop that
> > waits.
> 
> Then you have another side-effect, which is to delay the processing of
> those events.

But that's a problem we already have, since read-event doesn't return
them, right?  Or am I missing something?

> So instead of ignoring them it should process them.

If that's what read-event does, then yes.

> That's why I think we'd want to "simply" extract all that read-event
> does upto (not not including) the actual consumption (and return) of
> the event.

No sure I'm following you: the first thing read-event does is call
read_char, so how can we avoid reading events that way?

bug#21329: 25.0.50; Flyspell minor mode produces weird effects on keyboard macros

[Stefan Monnier]

>> >> One possible problem is that input-pending-p might return t in cases where
>> >> the pending input is "not significant", in the sense that read-event
>> >> won't return it (maybe because it will consume it internally as in the
>> >> case of event bounds to special-event-map, IIRC).
>> > I thought about ignoring out these kinds of input in the loop that
>> > waits.
>> Then you have another side-effect, which is to delay the processing of
>> those events.
> But that's a problem we already have, since read-event doesn't return
> them, right?  Or am I missing something?

AFAI, read-event does process those events bound in special-event-map
(and then keeps waiting for further events).

>> That's why I think we'd want to "simply" extract all that read-event
>> does upto (but not including) the actual consumption (and return) of
>> the event.
> No sure I'm following you: the first thing read-event does is call
> read_char, so how can we avoid reading events that way?

Right, we'd need to split read_char (and probably a good deal more).


        Stefan

bug#21329: 25.0.50; Flyspell minor mode produces weird effects on keyboard macros

[Stefan Monnier]

BTW, another way to fix this bug is to change flyspell so as not to use
sit-for like it does (which has other undesirable side-effects such as
delaying execution of post-command-hooks and such).


        Stefan