Add :general-config and add FAQ entry on avoiding performance issues

[noctuid]

Fixes #502.

• document which-key best practices
• document rewrite status (if have any replacement packages on MELPA, move the replaced functionality to a separate file that can be optionally not loaded)

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9651024) 69.40% compared to head (b1090d7) 69.19%.
Report is 3 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #503      +/-   ##
==========================================
- Coverage   69.40%   69.19%   -0.21%     
==========================================
  Files           1        1              
  Lines        1000     1003       +3     
==========================================
  Hits          694      694              
- Misses        306      309       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[blaenk]

Thank you so much for writing this and your continued work on general.el!

I think others will find it super helpful.

[blaenk]

By the way, I'm curious what you've used to measure startup time if you don't mind sharing. I've been out of the loop for a while and curious what options are out there. I was trying this fork of benchmark-init.el since the original was broken, but I don't recall if it was all that useful.

[noctuid]

Yes, I just used benchmark-init.

[tshu-w]

@noctuid Hi, If I bind keys in a prefix map, the which-key settings are not working.
current:

  (general-define-key
   :states '(normal insert motion emacs)
   :keymaps 'override
   :prefix-map 'tyrant-map
   :prefix "SPC"
   :non-normal-prefix "M-SPC")

  (general-create-definer tyrant-def :keymaps 'tyrant-map)
  (tyrant-def "" nil)
  (tyrant-def
    "b"       '(:ignore t :which-key "buffers")
    "bb"      'switch-to-buffer
    "bB"      'ibuffer
    "bd"      'kill-current-buffer
    "bm"      'switch-to-messages-buffer
    "bs"      'switch-to-scratch-buffer
    "bu"      'reopen-killed-buffer
    "bx"      'kill-buffer-and-window)

previous:

  (general-create-definer tyrant-def
    :states '(normal insert motion emacs)
    :keymaps 'override
    :prefix "SPC"
    :non-normal-prefix "M-SPC")
  (tyrant-def "" nil)
  (tyrant-def
    "b"       '(:ignore t :which-key "buffers")
    "bb"      'switch-to-buffer
    "bB"      'ibuffer
    "bd"      'kill-current-buffer
    "bm"      'switch-to-messages-buffer
    "bs"      'switch-to-scratch-buffer
    "bu"      'reopen-killed-buffer
    "bx"      'kill-buffer-and-window)

[noctuid]

Prefer not using :which-key wherever possible and instead use keymap-based replacements. In this case, I think you can bind "b" to something like (cons "buffers" (make-sparse-map)) instead of to '(:ignore t :which-key "buffers"). Try that and let me know if it works.

At some point I will add a new keyword :desc that will allow you to write "b" '(:desc "buffers"), but for now use my suggestion above or use which-key-add-keymap-based-replacements. Unlike :which-key, :desc will only accept a string replacement. If this is simple enough, I will add it to this PR.

I will update the readme to discourage using :which-key (if it doesn't already) unless you need a complex replacement. It is slower and less precise.

[tshu-w]

@noctuid (cons "buffers" (make-sparse-map)) works.

I have another question: I want to create a major mode prefix definer, how can I do that.
currently this is not working.

  (general-define-key
   :states '(normal insert motion emacs)
   :keymaps 'override
   :prefix-map 'despot-map
   :major-modes t
   :prefix "SPC m"
   :non-normal-prefix "M-SPC m")

  (general-create-definer despot-def :keymaps 'despot-map)
  (despot-def "" nil)

previous works:

  (general-create-definer despot-def
    :states '(normal insert motion emacs)
    :keymaps 'override
    :major-modes t
    :prefix "SPC m"
    :non-normal-prefix "M-SPC m")
  (despot-def "" nil)

[noctuid]

If I understand correctly, you will use this definer with different keymaps, in which case you should remove the :keymaps 'override. The suggestion to use a prefix map will only work for a single keymap. For a major-mode leader, you will need one prefix map per keymap. Try this (untested) to automatically generate the prefix keymaps:

(general-create-definer despot--prefix-def
  :states '(normal insert motion emacs)
  :prefix "SPC m"
  :non-normal-prefix "M-SPC m")

(defmacro despot-def (keymap &rest args)
  "Define keys using major-mode leader prefixes (\"SPC m\" or \"M-SPC m\").
KEYMAP should be an unquoted symbol. ARGS should be `general-def' args."
  (let ((prefix-map (intern (format "general-despot-%s" keymap))))
    `(progn
       (unless (boundp ',prefix-map)
         (despot--prefix-def ,keymap :prefix-map ',prefix-map))
       (general-def ,prefix-map ,@args))))

;; example usage; if you use a a syntax different from this, some changes to despot-def will be necessary
(despot-def emacs-lisp-mode-map "d" #'eval-defun)

This example code has some limitations/caveats. It will only work for a single keymap per use. Let me know if you use despot-def with multiple keymaps or a different syntax from the one here, and I can provide a better example.

I'll have to think about the best way to handle more complex use cases and add support for them. Eventually, there will be some way to opt-in to creating prefix maps automatically (e.g. can put :prefix-map t in a general-create-definer to automatically create/use a new prefix map based on the keymap name).

[tshu-w]

This example code has some limitations/caveats. It will only work for a single keymap per use. Let me know if you use despot-def with multiple keymaps or a different syntax from the one here, and I can provide a better example.

Thanks, sorry I was a little vague, but you got it. This example is enough.

[blaenk]

Thanks to @tshu-w for asking about the which-key prefixes as I actually had not noticed that the which-key labels weren't working anymore, and thanks @noctuid for providing the solution.

For anyone else who comes by reading this, note that it's actually (make-sparse-keymap) not (make-sparse-map) (ran into this myself).

Maybe it's something worth noting in the readme @noctuid as it's something that could easily be overlooked at the time when making these changes only to notice it later and not be super clear on what may have caused it.

[noctuid]

Yeah once I have some time I will update the documentation for these things and possibly add some helpers to simplify things.

I also forgot to mention that I also used profile-dotemacs, which is a lot more fine-grained/useful than benchmark-init for code that you have in your init (instead of just requires).

[blaenk]

Ah I see, thanks!

For others:

• https://www.emacswiki.org/emacs/ProfileDotEmacs
• Mirrored/ELPA: https://github.com/raxod502/profile-dotemacs

[tshu-w]

Hi, I'm facing another issue that I cannot define for minor-mode after using prefix-map:
currently:

  (general-define-key
   :states '(normal insert motion emacs)
   :keymaps 'override
   :prefix-map 'tyrant-map
   :prefix "SPC"
   :non-normal-prefix "M-SPC")

  (general-create-definer tyrant-def :keymaps 'tyrant-map)
  (tyrant-def "" nil)

  (tyrant-def eglot--managed-mode :definer 'minor-mode
    "ce"  (cons "eglot" (make-sparse-keymap))
    "cea" 'eglot-code-actions
    "ceb" 'eglot-events-buffer
    "cer" 'eglot-rename
    "ceR" 'eglot-reconnect
    "cex" 'eglot-shutdown
    "ceX" 'eglot-shutdown-all
    "ce=" 'eglot-format)

previous:

  (general-create-definer tyrant-def
    :states '(normal insert motion emacs)
    :keymaps 'override
    :prefix "SPC"
    :non-normal-prefix "M-SPC")
  (tyrant-def "" nil)
  (tyrant-def eglot--managed-mode :definer 'minor-mode
    "ce"  (cons "eglot" (make-sparse-keymap))
    "cea" 'eglot-code-actions
    "ceb" 'eglot-events-buffer
    "cer" 'eglot-rename
    "ceR" 'eglot-reconnect
    "cex" 'eglot-shutdown
    "ceX" 'eglot-shutdown-all
    "ce=" 'eglot-format)

[noctuid]

I think the issue is you're reusing your global/override-mode-map definer for non-global keybindings. This worked before because there was no :prefix-map but is still a little confusing. I assume you didn't bind c to anything else with tyrant-def? Otherwise, I don't think this would work at all.

You should create a separate definer that doesn't have :prefix-map. You can use the same strategy as despot-def to still use a prefix keymap.

Alternatively, if you never need to use SPC c for anything else, you can just bind in general-override-mode-map:

(tyrant-def
  :predicate '(bound-and-true-p eglot--managed-mode)
  :infix "c"
  "ea" #'eglot-code-actions
  ...)

The predicate isn't technically necessary but will prevent errors.

[tshu-w]

I forget to remove c when simplify bindings to show what I want.

You should create a separate definer that doesn't have :prefix-map. You can use the same strategy as despot-def to still use a prefix keymap.

I got it, thx a lot.