C-h b autoloads iso-transl and alters key-translation-map

C-h b autoloads iso-transl and alters key-translation-map

[David Reitter]

Bug #6527 describes how C-h b alters key bindings, namely alt- ones.

I have produced a minimal test case:

(map-keymap-internal 'ignore 'iso-transl-ctl-x-8-map)

(as called via `keymap_canonicalize')

or also

(keymap-parent  'iso-transl-ctl-x-8-map)

because it seems to be the call to get_keymap with autoloading that is causing it.


Indeed we have the following autoload:

;;;***
;;;### (autoloads nil "iso-transl" "international/iso-transl.el"
;;;;;;  (19423 17168))
;;; Generated autoloads from international/iso-transl.el
 (or key-translation-map (setq key-translation-map (make-sparse-keymap)))
 (define-key key-translation-map "\C-x8" 'iso-transl-ctl-x-8-map)
 (autoload 'iso-transl-ctl-x-8-map "iso-transl" "Keymap for C-x 8 prefix." t 'keymap)


Merely loading iso-trans.el is documented to create keybindings with Alt.

Should a package alter such central key bindings just upon loading it?
Or, should such a package be autoloaded?

Could people suggest a solution to this issue?

bug#6527: C-h b autoloads iso-transl and alters key-translation-map

[Stefan Monnier]

IIRC the reason why we've accepted the "change upon load" behavior of
iso-transl.el for so long is that it only affects Alt bindings, and most
Emacs users probably don't have both Meta and Alt keys, so they can't
get to the Alt bindings anyway.

This said, I think your patch is doing the right thing: move those extra
bindings to a minor mode.

I'd just like to see the implementation changed to make use of the new
multiple inheritance in keymaps:
- define the added bindings statically in a separate keymap (call it
  iso-transl-mode-translation-map).
- when enabling/disabling the mode, just add/remove that map from the
  parents of key-translation-map.
That will save you from the iso-transl-define-key dance of saving
previous bindings.


        Stefan

bug#6527: C-h b autoloads iso-transl and alters key-translation-map

[Chong Yidong]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> I'd just like to see the implementation changed to make use of the new
> multiple inheritance in keymaps:
> - define the added bindings statically in a separate keymap (call it
>   iso-transl-mode-translation-map).
> - when enabling/disabling the mode, just add/remove that map from the
>   parents of key-translation-map.

How about something like the following (modulo comment changes and
fixing the isearch integration)?

There's still no facility for cleanly adding/removing a keymap parent.
So this patch makes iso-transl-map the parent to key-translation-map at
top-level, and leaves it as the parent.  To enable the key translations,
it calls define-key on iso-transl-map; to disable the key translations,
it does (setcdr iso-transl-map nil), which turns iso-transl-map back
into a sparse keymap.  (Hence this relies on knowledge of the internal
representation of keymaps, but I don't see how to avoid that.)


=== modified file 'lisp/international/iso-transl.el'
*** lisp/international/iso-transl.el	2012-07-18 09:27:23 +0000
--- lisp/international/iso-transl.el	2012-07-18 11:20:36 +0000
***************
*** 236,249 ****
       ("N"  . [?Ñ])
       ("n"  . [?ñ]))))
  
! (defvar iso-transl-ctl-x-8-map nil
    "Keymap for C-x 8 prefix.")
! (or iso-transl-ctl-x-8-map
!     (fset 'iso-transl-ctl-x-8-map
! 	  (setq iso-transl-ctl-x-8-map (make-sparse-keymap))))
! (or key-translation-map
!     (setq key-translation-map (make-sparse-keymap)))
! (define-key key-translation-map "\C-x8" iso-transl-ctl-x-8-map)
  
  ;; For each entry in the alist, we'll make up to three ways to generate
  ;; the character in question: the prefix `C-x 8'; the ALT modifier on
--- 236,247 ----
       ("N"  . [?Ñ])
       ("n"  . [?ñ]))))
  
! (defvar iso-transl-map (make-sparse-keymap))
! (set-keymap-parent key-translation-map iso-transl-map)
! 
! (defvar iso-transl-ctl-x-8-map (make-sparse-keymap)
    "Keymap for C-x 8 prefix.")
! (fset 'iso-transl-ctl-x-8-map iso-transl-ctl-x-8-map)
  
  ;; For each entry in the alist, we'll make up to three ways to generate
  ;; the character in question: the prefix `C-x 8'; the ALT modifier on
***************
*** 259,265 ****
  	    (vec (vconcat (car (car alist))))
  	    (tail iso-transl-dead-key-alist))
  	(aset vec 0 (logior (aref vec 0) ?\A-\^@))
! 	(define-key key-translation-map vec translated-vec)
  	(define-key isearch-mode-map (vector (aref vec 0)) nil)
  	(while tail
  	  (if (eq (car (car tail)) inchar)
--- 257,263 ----
  	    (vec (vconcat (car (car alist))))
  	    (tail iso-transl-dead-key-alist))
  	(aset vec 0 (logior (aref vec 0) ?\A-\^@))
! 	(define-key iso-transl-map vec translated-vec)
  	(define-key isearch-mode-map (vector (aref vec 0)) nil)
  	(while tail
  	  (if (eq (car (car tail)) inchar)
***************
*** 267,273 ****
  		    (deadkey (cdr (car tail))))
  		(aset deadvec 0 deadkey)
  		(define-key isearch-mode-map (vector deadkey) nil)
! 		(define-key key-translation-map deadvec translated-vec)))
  	  (setq tail (cdr tail)))))
      (setq alist (cdr alist))))
  
--- 265,271 ----
  		    (deadkey (cdr (car tail))))
  		(aset deadvec 0 deadkey)
  		(define-key isearch-mode-map (vector deadkey) nil)
! 		(define-key iso-transl-map deadvec translated-vec)))
  	  (setq tail (cdr tail)))))
      (setq alist (cdr alist))))
  
***************
*** 280,286 ****
  
  ;; The standard mapping comes automatically.  You can partially overlay it
  ;; with a language-specific mapping by using `M-x iso-transl-set-language'.
! (iso-transl-define-keys iso-transl-char-map)
  
  (provide 'iso-transl)
  
--- 278,295 ----
  
  ;; The standard mapping comes automatically.  You can partially overlay it
  ;; with a language-specific mapping by using `M-x iso-transl-set-language'.
! 
! (define-minor-mode iso-transl-mode
!   "Toggle ISO-8859 Key Translation mode."
!   :group 'i18n
!   :global t
!   (setcdr iso-transl-map nil)
!   (setcdr iso-transl-ctl-x-8-map nil)
!   (when iso-transl-mode
!     (iso-transl-define-keys iso-transl-char-map)
!     (define-key iso-transl-map "\C-x8" iso-transl-ctl-x-8-map)))
! 
! (iso-transl-mode)
  
  (provide 'iso-transl)
  

bug#6527: C-h b autoloads iso-transl and alters key-translation-map

[Stefan Monnier]

> How about something like the following (modulo comment changes and
> fixing the isearch integration)?

I don't much like the idea of hard-coding key-translation-map's parent
like this.

> There's still no facility for cleanly adding/removing a keymap parent.

Then, let's try to fix this problem.
How 'bout:

  (defun add-parent (map new-parent)
    (let ((cur (keymap-parent map)))
      (cond
       ((null cur) (set-keymap-parent map new-parent))
       ((eq cur new-parent))
       ((and (consp cur) (memq new-parent cur)))
       ((implicit-parents-holder-p cur)
        (push new-parent (cdr cur)))
       (t (set-keymap-parent
           map (make-composed-keymap (list new-parent cur)))))))

  (defun remove-parent (map parent)
    (let ((cur (keymap-parent map)))
      (cond
       ((eq cur parent) (set-keymap-parent map nil))
       ((and (consp cur) (memq parent cur))
        (delq parent cur)
        (unless (cdr cur) (set-keymap-parent map nil))))))

The only remaining problem is to define implicit-parents-holder-p
which magically determines if the keymap is one of those that where
implicitly created by add-parent (by calling make-composed-keymap)
or if it's a *real* keymap that belongs to someone else and that we
shouldn't modify.

        
        Stefan