GNU bug report logs - #14095
24.3.50; REGRESSION: `repeat' broken by use of `set-temporary-overlay-map'

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 14095 in the body.
You can then email your comments to 14095 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: "Drew Adams" <drew.adams <at> oracle.com>
To: <bug-gnu-emacs <at> gnu.org>
Subject: 24.3.50;
	REGRESSION: `repeat' broken by use of `set-temporary-overlay-map'
Date: Sat, 30 Mar 2013 10:13:21 -0700

I believe this regression was introduced at the same time as the
regression of bug #12232 (which has been fixed).  The change that
introduced this problem was to use `set-temporary-overlay-map'.
 
The code for `repeat' before that change, e.g. the code from Emacs 23.4,
had this:
 
(when repeat-repeat-char
  (let (repeat-on-final-keystroke
        (undo-inhibit-record-point t))
    (setq real-last-command 'repeat)
    (setq repeat-undo-count 1)
    (unwind-protect
        (while (let ((evt (read-key)))
                 (eq (or (car-safe evt) evt)
                     (or (car-safe repeat-repeat-char)
                         repeat-repeat-char)))
          (repeat repeat-arg))
      ;; Make sure `repeat-undo-count' is reset.
      (setq repeat-undo-count nil))
    (setq unread-command-events (list last-input-event))))
 
That was changed to this (the current code):
 
(when repeat-repeat-char
  (set-temporary-overlay-map ; <===================
   (let ((map (make-sparse-keymap)))
     (define-key map (vector repeat-repeat-char)
       (if (null repeat-message-function) 'repeat
         (let ((fun repeat-message-function))
           (lambda ()
             (interactive)
             (let ((repeat-message-function fun))
               (setq this-command 'repeat)
               (setq real-this-command 'repeat)
               (call-interactively 'repeat))))))
     map)))
 
This change breaks the use of `repeat' in the following context, because
Isearch uses `overriding-terminal-local-map'.  Recipe:
 
emacs -Q

(defun isearchp-yank-line ()
    "Yank text from buffer up to end of line onto search string.
You can repeat this by hitting the last key again..."
    (interactive)
    (require 'repeat nil t)
    (isearchp-repeat-command 'isearch-yank-line))

(defun isearchp-repeat-command (command)
  "Repeat COMMAND."
  (let ((repeat-message-function  'ignore))
    (setq last-repeatable-command  command)
    (repeat nil)))

(define-key isearch-mode-map (kbd "C-y C-e")
            'isearchp-yank-line)
 
C-s  C-y  C-e  C-e  C-e
 
That works if the old definition of `repeat' is used.  It does not work
with the new definition, which uses `set-temporary-overlay-map', because
the temporary map is overruled by `overriding(-terminal)-local-map'.
 
Users should be able to use `repeat' in contexts such as this.  For
Isearch, for example, this lets you put multiple yank commands on the
same prefix key (e.g., `C-y').
 
For instance, I bind 8 yank commands to prefix C-y, 5 of which are
repeatable (i.e., they are repeatable in Emacs 22-23 and should also
be repeatable in later releases):
 
 C-y C-_   isearchp-yank-symbol-or-char
 C-y C-(   isearchp-yank-sexp-symbol-or-char
 C-y C-2   isearch-yank-secondary
 C-y C-c   isearchp-yank-char
 C-y C-e   isearchp-yank-line
 C-y C-w   isearchp-yank-word-or-char
 C-y C-y   isearch-yank-kill
 C-y M-y   isearch-yank-pop
 
(FWIW/FYI: It took me quite a while to figure out why this was not working.)

In GNU Emacs 24.3.50.1 (i386-mingw-nt5.1.2600)
 of 2013-03-23 on VBOX
Bzr revision: 112115 eliz <at> gnu.org-20130323093300-rjs0dgskxm9u0ya4
Windowing system distributor `Microsoft Corp.', version 5.1.2600
Configured using:
 `configure --with-gcc (4.7) --no-opt --enable-checking --cflags
 -IC:/emacs/libs/libXpm-3.5.10/include -IC:/emacs/libs/libXpm-3.5.10/src
 -IC:/emacs/libs/libpng-dev_1.4.3-1_win32/include
 -IC:/emacs/libs/zlib-dev_1.2.5-2_win32/include
 -IC:/emacs/libs/giflib-4.1.4-1-lib/include
 -IC:/emacs/libs/jpeg-6b-4-lib/include
 -IC:/emacs/libs/tiff-3.8.2-1-lib/include
 -IC:/emacs/libs/libxml2-2.7.8-w32-bin/include/libxml2
 -IC:/emacs/libs/gnutls-3.1.10-w32/include
 -IC:/emacs/libs/libiconv-1.14-2-mingw32-dev/include'
 

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: "Drew Adams" <drew.adams <at> oracle.com>
Cc: 14095-done <at> debbugs.gnu.org
Subject: Re: bug#14095: 24.3.50;
 REGRESSION: `repeat' broken by use of `set-temporary-overlay-map'
Date: Thu, 13 Jun 2013 16:44:15 -0400

> C-s  C-y  C-e  C-e  C-e
> That works if the old definition of `repeat' is used.  It does not work

This should work now, thank you,


        Stefan

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 14095-done <at> debbugs.gnu.org
Subject: RE: bug#14095: 24.3.50; REGRESSION: `repeat' broken by use of
 `set-temporary-overlay-map'
Date: Thu, 13 Jun 2013 20:42:04 -0700 (PDT)

> > C-s  C-y  C-e  C-e  C-e
> > That works if the old definition of `repeat' is used.  It does not work
> 
> This should work now, thank you,

I want to say thank you!  But unfortunately there is still a problem,
in this build (which I believe includes your fix):

In GNU Emacs 24.3.50.1 (i686-pc-mingw32)
 of 2013-06-13 on ODIEONE
Bzr revision: 112978 xfq.free <at> gmail.com-20130613224333-3yfl8navh3c1vmxy
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=/c/Devel/emacs/binary --enable-checking=yes,glyphs
 CFLAGS='-O0 -g3' CPPFLAGS='-Ic:/Devel/emacs/include'
 LDFLAGS='-Lc:/Devel/emacs/lib''

From `emacs -Q', I do this:

(defun isearchp-repeat-command (command)
  "..."
  (let ((repeat-message-function  'ignore))
    (setq last-repeatable-command  command)
    (repeat nil)))

(defun isearchp-yank-line ()
  "..."
  (interactive)
  (require 'repeat nil t)
  (isearchp-repeat-command 'isearch-yank-line))

(define-key isearch-mode-map "\C-y" nil)
(define-key isearch-mode-map (kbd "C-y C-e") 'isearchp-yank-line)

Things seem to work pretty much as they should, but if you do
`C-s C-y C-e C-e' then as soon as you hit the second `C-e' you
see this error msg: "Stack overflow in equal", which comes from
`set-temporary-overlay-map'.

With non-nil `debug-on-error' you get a backtrace like this:

Debugger entered--Lisp error: (error "Stack overflow in equal")
  add-hook(pre-command-hook #[0 "... [(keymap (5 . #[0 "..."
   [ignore repeat-message-function this-command real-this-command
    repeat call-interactively] 3 "\n\n(fn)" nil])) nil nil (#0)
    this-command nil t lookup-key this-command-keys-vector
    remove-hook pre-command-hook internal-pop-keymap
    overriding-terminal-local-map] 4 "\n\n(fn)"])

(I copied that backtrace from a session using my setup, not
emacs -Q, but it looks, superficially at least, the same as what
I see with emacs -Q.)

Hope this helps.  I will leave the bug closed for now.  Thanks
for working on this.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 14095-done <at> debbugs.gnu.org
Subject: Re: bug#14095: 24.3.50;
 REGRESSION: `repeat' broken by use of `set-temporary-overlay-map'
Date: Fri, 14 Jun 2013 00:14:17 -0400

> Things seem to work pretty much as they should, but if you do
> `C-s C-y C-e C-e' then as soon as you hit the second `C-e' you
> see this error msg: "Stack overflow in equal", which comes from
> `set-temporary-overlay-map'.

Good point, thanks.  I've installed a further fix (see below) which
should fix this problem.


        Stefan


=== modified file 'lisp/subr.el'
--- lisp/subr.el	2013-06-13 22:24:52 +0000
+++ lisp/subr.el	2013-06-14 04:08:45 +0000
@@ -3794,12 +3794,15 @@
                  (if (not load-file-name)
                      ;; Not being provided from a file, run func right now.
                      (funcall func)
-                   (let ((lfn load-file-name))
-                     (letrec ((fun (lambda (file)
+                   (let ((lfn load-file-name)
+                         ;; Don't use letrec, because equal (in
+                         ;; add/remove-hook) would get trapped in a cycle.
+                         (fun (make-symbol "eval-after-load-helper")))
+                     (fset fun (lambda (file)
                                      (when (equal file lfn)
                                        (remove-hook 'after-load-functions fun)
-                                       (funcall func)))))
-                       (add-hook 'after-load-functions fun))))))))
+                                   (funcall func))))
+                     (add-hook 'after-load-functions fun)))))))
         ;; Add FORM to the element unless it's already there.
         (unless (member delayed-func (cdr elt))
           (nconc elt (list delayed-func)))))))
@@ -4282,7 +4285,10 @@
 
 Optional ON-EXIT argument is a function that is called after the
 deactivation of MAP."
-  (letrec ((clearfun
+  (let ((clearfun (make-symbol "clear-temporary-overlay-map")))
+    ;; Don't use letrec, because equal (in add/remove-hook) would get trapped
+    ;; in a cycle.
+    (fset clearfun
             (lambda ()
               ;; FIXME: Handle the case of multiple temporary-overlay-maps
               ;; E.g. if isearch and C-u both use temporary-overlay-maps, Then
@@ -4298,7 +4304,7 @@
                             (t (funcall keep-pred)))
                 (remove-hook 'pre-command-hook clearfun)
                 (internal-pop-keymap map 'overriding-terminal-local-map)
-                (when on-exit (funcall on-exit))))))
+              (when on-exit (funcall on-exit)))))
     (add-hook 'pre-command-hook clearfun)
     (internal-push-keymap map 'overriding-terminal-local-map)))
 

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 14095-done <at> debbugs.gnu.org
Subject: RE: bug#14095: 24.3.50; REGRESSION: `repeat' broken by use of
 `set-temporary-overlay-map'
Date: Wed, 19 Jun 2013 19:24:35 -0700 (PDT)

> Good point, thanks.  I've installed a further fix (see below) which
> should fix this problem.

Yes, the problem is fixed.  Thx.

Previous Next

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