search

ch11ng / exwm

Emacs X Window Manager
2.85k stars 134 forks source link

Please review `:value-type` of `exwm-input-global-keys` & other related thoughts concerning that option #635

Closed emacksnotes closed 5 years ago

emacksnotes commented 5 years ago

Please review :value-type of exwm-input-global-keys

Suggested fix

Make the :value-type a sexp or a restricted-sexp that tests for keymapp.

Only affects users that rely on defcustom widget to configure exwm-input-global-keys.

If you are a user who directly setq-s the user option, then my complaint will make no sense to you. Since the variable has a set-tter, using a setq will be of little use in a running session anyways.

The note below outlines what forced me to my turn my radar on exwm-input-global-keys. My hope is that the note below will be of use to others.

(The rest of the report is more a conversation, than an issue per-se.)

TLDR

The :set function of exwm-input-global-keys ultimately calls

  (define-key (current-global-map) key command)

The 3rd arg of define-key need not be a command, but it can be a keymap too.

Currently exwm-input-global-keys restricts :value-type to a function, and it is too limiting. Specifically it wouldn't allow me to specify a keymap for the :valuetype.

**define-key is a built-in function in ‘keymap.c’.**

**(define-key KEYMAP KEY DEF)**

  Probably introduced at or before Emacs version 13.

In KEYMAP, define key sequence KEY as DEF.
KEYMAP is a keymap.

KEY is a string or a vector of symbols and characters, representing a
sequence of keystrokes and events.  Non-ASCII characters with codes
above 127 (such as ISO Latin-1) can be represented by vectors.
Two types of vector have special meanings:
 [remap COMMAND] remaps any key binding for COMMAND.
 [t] creates a default definition, which applies to any event with no
    other definition in KEYMAP.

**DEF is anything that can be a key’s definition:**
 nil (means key is undefined in this keymap),
 a command (a Lisp function suitable for interactive calling),
 a string (treated as a keyboard macro),
 **a keymap (to define a prefix key),**
 a symbol (when the key is looked up, the symbol will stand for its
    function definition, which should at that time be one of the above,
    or another symbol whose function definition is used, etc.),
 a cons (STRING . DEFN), meaning that DEFN is the definition
    (DEFN should be a valid definition in its own right),
 or a cons (MAP . CHAR), meaning use definition of CHAR in keymap MAP,
 or an extended menu item definition.
 (See info node ‘(elisp)Extended Menu Items’.)

If KEYMAP is a sparse keymap with a binding for KEY, the existing
binding is altered.  If there is no binding for KEY, the new pair
binding KEY to DEF is added at the front of KEYMAP.

[back]

Motivation

I prefer putting non-Emacs buffers like Firefox or Gnome-Terminal etc in char-mode. This way I can continue using those apps as I have always used them in ancient times.

Whenever I work with these char mode buffers within EXWM, I want to be able to C-x b C-x C-f C-x d or invoke many of the other buffer, window and frame commands in ctl-x-map.

If I were to make the above ctl-x-map commands actionable in char mode buffers, I need to "migrate" those commands to exwm-input-global-keys in single "swoop". And the easiest way to do it is to do something like

Define a keymap widget

(define-widget 'keymap 'restricted-sexp
  "A keymap."
  :completions (apply-partially #'completion-table-with-predicate
                                obarray #'keymapp 'strict)
  :prompt-value 'widget-field-prompt-value
  :prompt-internal 'widget-symbol-prompt-internal
  :prompt-match 'keymapp
  :prompt-history 'widget-function-prompt-value-history
  :action 'widget-field-action
  :match-alternatives '(keymapp)
  :validate (lambda (widget)
          (unless (keymapp (widget-value widget))
        (widget-put widget :error (format "Invalid keymap: %S"
                          (widget-value widget)))
        widget))
  :value '(keymap (t . nil))
  :tag "Keymap")

Note: keymap deserves a widget of it's own. Stock Emacs doesn't provide one. I am providing this widget only so that it is for you to try and test the validity and usefulness of my suggestion here.

Use the keymap widget type

(defcustom exwm-input-global-keys nil
  "Global keys.

It is an alist of the form (key . command), meaning giving KEY (a key
sequence) a global binding as COMMAND.

Notes:
* Setting the value directly (rather than customizing it) after EXWM
  finishes initialization has no effect."
  :type '(alist :key-type key-sequence :value-type (choice
                            (function :tag "Function" )
                            (keymap :tag "Keymap")))
  :set (lambda (symbol value)
         (when (boundp symbol)
           (dolist (i (symbol-value symbol))
             (global-unset-key (car i))))
         (set symbol value)
         (setq exwm-input--global-keys nil)
         (dolist (i value)
           (exwm-input--set-key (car i) (cdr i)))
         (when exwm--connection
           (exwm-input--update-global-prefix-keys))))

Once that is done, I can have customization like this

(custom-set-variables
 '(exwm-input-global-keys
   '(([8388728]
      . Control-X-prefix))))

With that in no time I would be able to do s-x b, s-x C-f, s-x d, s-x 1 etc. when in char mode buffers too.

Note that Control-X-Prefix used above was only an example. It could be any keymap, either well-known or even a user-defined one. I have used Control-X-Prefix only as an example. It is available to everyone using Emacs, and someone reading this note can test it immediately. The keymap here need not be a well-known one. I anticipate that in most cases will be a user-defined one.

How I hope to use the suggested feature

For example, I have bound all the window management commands in a sparse map which is bound to C-x w key. Now with the suggestion above, it is child's game to move over my C-x w map to s-w map, for example.

(defvar my-window-mode-map
  (let ((map (make-sparse-keymap "Window")))
    (cl-loop for (string . command) in
         '(
           ;; Enlarge window horizontally
           ("<right> +"     . my-enlarge-window-horizontally)
           ("<left> +"      . my-enlarge-window-horizontally)
           ("+ <right>"     . my-enlarge-window-horizontally)
           ("+ <left>"      . my-enlarge-window-horizontally)
           ("<right> ="     . my-enlarge-window-horizontally)
           ("<left> ="      . my-enlarge-window-horizontally)
           ("= <right>"     . my-enlarge-window-horizontally)
           ("= <left>"      . my-enlarge-window-horizontally)
           ;; Shrink window horizontally
           ("<right> -"     . my-shrink-window-horizontally)
           ("<left> -"      . my-shrink-window-horizontally)
           ("- <right>"     . my-shrink-window-horizontally)
           ("- <left>"      . my-shrink-window-horizontally)
           ;; Enlarge window vertically
           ("<up> +"        . my-enlarge-window)
           ("<down> +"      . my-enlarge-window)
           ("+ <up>"        . my-enlarge-window)
           ("+ <down>"      . my-enlarge-window)
           ("<up> ="        . my-enlarge-window)
           ("<down> ="      . my-enlarge-window)
           ("= <up>"        . my-enlarge-window)
           ("= <down>"      . my-enlarge-window)
           ;; Shrink window vertically
           ("<up> -"        . my-shrink-window)
           ("<down> -"      . my-shrink-window)
           ("- <up>"        . my-shrink-window)
           ("- <down>"      . my-shrink-window)
           ;; Maximize window
           ("= ="       . maximize-window)
           ("+ +"       . maximize-window)
           ("- -"       . minimize-window)
           ;; Minimize window
           ;; ("= h"        . my-enlarge-window)
           ;; ("- -"        . minimize-window)
           ;; Split window below
           ("<down> <down>" . my-split-window-below)
           ("<up> <up>"     . my-split-window-below)
           ;; Split window right
           ("<right> <right>"   . my-split-window-right)
           ("<left> <left>" . my-split-window-right)
           ("t" . window-swap-states)
           ;; Winner Undo and Redo
           ("/" . winner-undo)
           ("?" . winner-redo)
           ;; Balance Windows
           ("0" . balance-windows))
         do ;; (global-set-key (kbd (concat "C-x w " string))

         ;;             command)

         (define-key map (kbd string) command)
         ;; (define-key global-map (kbd (concat "s-w " string)) command)
         )
    map)
  "Key map for My Window Mode.")

(defalias 'my-window-mode-prefix my-window-mode-map)
(define-key global-map (kbd "C-x w") 'my-window-mode-prefix)

With that the value of exwm-input-global-keys can have the following entry

(([8388727]
  . my-window-mode-prefix))

and I will be able to use s-w to manage the windows.

Advantage: "migrate" a set of commands mapped to a prefix to a exwm-only prefix.

In the above example, I migrated C-x w commands to s-w commands in a single swoop. Such wholesale migration not possible if the :value-type of exwm-input-global-keys is restricted to a function.

More note

I have more ideas in regard to exwm-input-global-keys, and I will use this issue tracker to record them.

ch11ng commented 5 years ago

A better way to integrate X windows into Emacs is to use the line-mode, which allows users to use nearly every keybindings they want and translate inconsistent keybindings with simulation keys. That said, there should be an easy way to access a bunch of keybindings in char-mode. I believe the following command can do the work (if we publish it as an public interface):

(defun invoke-ctl-x-map ()
  (interactive)
  (exwm-input--cache-event ?\C-x t)
  (exwm-input--unread-event ?\C-x))
emacksnotes commented 5 years ago

A better way to integrate X windows into Emacs is to use the line-mode, which allows users to use nearly every keybindings they want and translate inconsistent keybindings with simulation keys. That said, there should be an easy way to access a bunch of keybindings in char-mode. I believe the following command can do the work (if we publish it as an public interface):

(defun invoke-ctl-x-map ()
  (interactive)
  (exwm-input--cache-event ?\C-x t)
  (exwm-input--unread-event ?\C-x))

I have been experimenting with the above suggestion. This is what I have come up with

(defmacro exwm--wrap-event (arg)
  (let* ((evlist (eval `(listify-key-sequence ,arg)))
     (ev (elt evlist 0)))
    `(defun ,(intern (format "exwm--invoke-%s-map" (key-description evlist))) ()
       ,(format "Invoke `%s' map zzz." (key-description evlist))
       (interactive)
       (exwm-input--cache-event ,ev t)
       (exwm-input--unread-event ,ev))))

(exwm--wrap-event (kbd "C-h"))

(exwm--wrap-event (kbd "C-x"))

(exwm--wrap-event (kbd "M-x"))

;; exwm-input-prefix-keys
;; '(?\C-x ?\C-u ?\C-h ?\M-x ?\M-` ?\M-& ?\M-:)

(custom-set-variables
 '(exwm-input-global-keys
   '(([33554456]            ; "C-S-x"
      . exwm--invoke-C-x-map)
     ([33554440]            ; "C-S-h"
      . exwm--invoke-C-h-map)
     ([134217816]           ; "M-X"
      . exwm--invoke-M-x-map))))

It is easier to migrate C-x to C-X, C-h to C-H and M-x to M-X. For regular use, having the commands in ctl-x-map is the most useful in EXWM buffers. Since I am actively exploring EXWM, and running in to issues I have found a need for having access to C-h and M-x commands.

That said ... 

(defun invoke-ctl-x-map ()
  (interactive)
  (exwm-input--cache-event ?\C-x t)
  (exwm-input--unread-event ?\C-x))

I wonder what would be the recipe, when the prefix key is not a single key but multiple keys.

Put in other words, the example above assumes a ctl-x-map, what if one wants to "migrate" a set of commands say in a (hypothetical) ctl-x-ctl-y-map.

If you notice carefully, my macro which does event->command mapping assumes that event is a vector with just a single key.

emacksnotes commented 5 years ago

I had to do a lot of trial error to genericize 

(defun invoke-ctl-x-map ()
  (interactive)
  (exwm-input--cache-event ?\C-x t)
  (exwm-input--unread-event ?\C-x))

to 

(defmacro exwm--wrap-event (arg)
  (let* ((evlist (eval `(listify-key-sequence ,arg)))
     (ev (elt evlist 0)))
    `(defun ,(intern (format "exwm--invoke-%s-map" (key-description evlist))) ()
       ,(format "Invoke `%s' map zzz." (key-description evlist))
       (interactive)
       (exwm-input--cache-event ,ev t)
       (exwm-input--unread-event ,ev))))

In the numerous trials I leading up to the above macro, I got the /type/ of the first parameter of exwm-input--cache-event and exwm-input--unread-event wrong. EVENT in your suggestion is actually a number. And it took some time to figure this fact out. i.e., I misread ?\C-x as a string, but is in fact an integer. My brain interpolated "double quotes" in what /appeared/ like a string.

Here is the MOST IMPORTANT part.

When I got the type of the EVENT parameter wrong, not just the EXWM but the whole GNOME session went for a toss. There were numerous instances where my laptop stopped responding to /any/ key & mouse, and I had to forcibly power cycle the laptop. At one point I was even worried about my hard disk getting broke.

To cut the long story short, here is my request. And it is simple ...

Any user error that may happen in the above path should be demoted, and EXWM should do it's best to remain in the working state.

The bug description is vague, but I hope the maintainers can figure out what needs to be done.

cf. Gracefully handle xmodmap -e "clear mod4" · Issue #655 · ch11ng/exwm

This theme of "Graceful handling of user errors" came up (and subsequently fixed) in https://github.com/ch11ng/exwm/issues/612#issuecomment-526913074 too. So, I think it would be a good idea to review the code path that gets triggered from user's custom-set-variables and ensure that any fault on user's part doesn't result in an unusable system. Unless one is an expert user, getting Emacs key sequences right is an arduous task, and there is a high probability that one gets the syntax wrong rather than right.

ch11ng commented 5 years ago

Put in other words, the example above assumes a ctl-x-map', what if one wants to "migrate" a set of commands say in a (hypothetical)ctl-x-ctl-y-map'.

That's quite straightforward:

(defun invoke-ctl-x-ctl-y-map ()
  (interactive)
  (exwm-input--cache-event ?\C-x t)
  (exwm-input--unread-event ?\C-x)
  (exwm-input--cache-event ?\C-y t)
  (exwm-input--unread-event ?\C-y))

Basically a key sequence can be represented as a vector a key event(s). exwm-input--cache-event and exwm-input--unread-event deal with key events so just feed them with each key event in order.

Any user error that may happen in the above path should be demoted, and EXWM should do it's best to remain in the working state.

I'm afraid there's no generic way but to proofread all codes, as most X requests have side effects (it's hard to rollback). But I'll try. That specific problem is fixed of course.

emacksnotes commented 5 years ago

I prefer putting non-Emacs buffers like Firefox or Gnome-Terminal etc in char-mode. This way I can continue using those apps as I have always used them in ancient times.

Whenever I work with these char mode buffers within EXWM, I want to be able to C-x b' .C-x C-f', or invoke many of the other buffer, window and frame commands in ctl-x-map.

If I were to make the above ctl-x-map commands actionable in char mode buffers, I need to "migrate" those commands to exwm-input-global-keys in single "swoop".

A better way to integrate X windows into Emacs is to use the line-mode, which allows users to use nearly every keybindings they want and translate inconsistent keybindings with simulation keys. That said, there should be an easy way to access a bunch of keybindings in char-mode. I believe the following command can do the work (if we publish it as an public interface):

(defun invoke-ctl-x-map () (interactive) (exwm-input--cache-event ?\C-x t) (exwm-input--unread-event ?\C-x))

(defun invoke-ctl-x-ctl-y-map () (interactive) (exwm-input--cache-event ?\C-x t) (exwm-input--unread-event ?\C-x) (exwm-input--cache-event ?\C-y t) (exwm-input--unread-event ?\C-y))

This is what I have right now:

(defmacro exwm--with-gensyms (symbols &rest body)
  "Same as `org-with-gensyms.'"
  (declare (debug (sexp body)) (indent 1))
  `(let ,(mapcar (lambda (s)
           `(,s (make-symbol (concat "--" (symbol-name ',s)))))
                 symbols)
     ,@body))

(defmacro exwm--invoke-map (kbd)
  `(eval ,(exwm--with-gensyms (evlist keydesc)
        `(let* ((,evlist (listify-key-sequence ,kbd))
            (,keydesc (key-description ,evlist)))
           `(defun ,(intern (format "exwm--invoke-%s-map" ,keydesc)) ()
          ,(format "Invoke `%s' map." ,keydesc)
          (interactive)
          ,@(cl-loop for event in ,evlist
                 append `((exwm-input--cache-event ,event t)
                      (exwm-input--unread-event ,event))))))))

(customize-set-variable
 'exwm-input-global-keys
 (append exwm-input-global-keys
     (cl-loop for (from-keys . to-keys) in '(("C-x" . "C-S-x")
                         ("C-h" . "C-S-h")
                         ("M-x" . "M-X")
                         ("M-:" . "C-M-:")
                         ;; ("C-x w" . "s-w")
                         )
          collect (cons (vconcat (listify-key-sequence (kbd to-keys)))
                (exwm--invoke-map (kbd from-keys))))))

With the above snippets in .emacs, this is what M-: exwm-input-global-keys RET looks like

(([33554456]
  . exwm--invoke-C-x-map)
 ([33554440]
  . exwm--invoke-C-h-map)
 ([134217816]
  . exwm--invoke-M-x-map)
 ([201326650]
  . exwm--invoke-M-:-map)
 ([8388727]
  . exwm--invoke-C-x\ w-map))

Note the following:

  1. For Emacs commands that are common across char-mode and line-mode buffers, all I need to is press shift.
  2. It supports "multiple" prefix keys.
emacksnotes commented 5 years ago

I believe the following command can do the work (if we publish it as an public interface)

Please make the interface public. I will be a happy consumer.

The macro exwm--invoke-map that you see in the previous message, gives you a hint on what /form/ the public interface could be i.e., it should work with multiple prefix keys. It /may/ install a /invoke function/ in runtime, and have it return a /name/ for it (as opposed to it returning a anonymous function). With a named function, the custom-set-variable blob in .emacs will be short and comprehensible.

emacksnotes commented 5 years ago

The macro exwm--invoke-map that you see in the previous message, gives you a hint on what /form/ the public interface could be i.e., it should work with multiple prefix keys.

When given a multiple prefix keys, you /may/ want to check for emacs-version,

I compile Emacs from git, so I would very much want to see the above interface take multiple key events.

ch11ng commented 5 years ago

I've added this as exwm-input-invoke-factory. Please check it out. One downside of this is the resulting name can contain strange characters like space, but it's probably acceptable for display purpose only. Note that I didn't do the check against Emacs version as 26.3 is already released.

emacksnotes commented 5 years ago

I've added this as exwm-input-invoke-factory.

Thanks. I am closing this bug.

Usage Note 1

This is what I have in my .emacs.

(customize-set-variable
 'exwm-input-global-keys
 (append exwm-input-global-keys
     (cl-loop for (from-keys . to-keys) in '(("C-x"     . "C-S-x"   )
                         ("C-h"     . "C-S-h"   )
                         ("M-x"     . "M-X"     )
                         ("M-:"     . "C-M-:"   )
                         ;; ("C-x w"    . "s-w"     )
                         )
          collect (cons (vconcat (listify-key-sequence (kbd to-keys)))
                (eval `(exwm-input-invoke-factory ,from-keys))))))

Usage Note 2

One can also go with the following in their .emacs.

(exwm-input-invoke-factory "C-x"    ) ; exwm-input--invoke--C-x
(exwm-input-invoke-factory "C-h"    ) ; exwm-input--invoke--C-h
(exwm-input-invoke-factory "M-x"    ) ; exwm-input--invoke--M-x
(exwm-input-invoke-factory "M-:"    ) ; exwm-input--invoke--M-:
(exwm-input-invoke-factory "C-x w"  ) ; exwm-input--invoke--C-x\ w

(custom-set-variables
 '(exwm-input-global-keys
   '(([33554456]
      . exwm-input--invoke--C-x)
     ([33554440]
      . exwm-input--invoke--C-h)
     ([134217816]
      . exwm-input--invoke--M-x)
     ([201326650]
      . exwm-input--invoke--M-:)
     ([8388727]
      . exwm-input--invoke--C-x\ w))))