Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite #497

Open
noctuid opened this issue Sep 29, 2021 · 16 comments
Open

Rewrite #497

noctuid opened this issue Sep 29, 2021 · 16 comments

Comments

@noctuid
Copy link
Owner

noctuid commented Sep 29, 2021

I wrote a PoC a while ago with some minimal functionality (keymap/state aliases, :prefix handling, prefix map creation, auto kbd, etc.). Once I've written tests and cleaned it up, I will create a branch for it. There is no timeline for this to become stable and replace general.el. This is not a priority for me since general.el works, and I don't think there's a huge interest in this, so it might be many years before that happens.

Summary of changes/tl;dr

  • cleaner internal implementation and extension system
  • more powerful extension system (internals implemented pretty much entirely through extension system)
  • performance gotchas addressed
  • better documentation
  • leto.el - macro to expand to definer statements to minimal, human-readable code (for transparency to show what a definer actually does)
  • more alternate definition syntaxes
  • single definer recommended for most situations that doesn't have the caveats of general-def; short name (just the new package name)
  • more non-keybinding configuration helpers (in separate file)

Why rewrite?

  • Simpler implementation. General was not so general in the beginning and has special handling for a lot of functionality (especially evil support being hardcoded as opposed to implemented using the extension system). The general.el internals should be generic too.
    • Almost all functionality in the rewrite will be implemented as a linear sequence of transformations over keyword arguments and then keybindings.
    • The extension system will be the same as the internal implementation. The only difference will be that helpers will be provided to insert keyword argument and keybinding parsers in the correct order in a forwards-compatible way (since parsers can transform/add new keybindings or generate metadata, order will matter)
    • Transformations can alter keyword arguments (e.g. to expand keymap aliases) and keybindings (e.g. to apply kbd to the key). They can add new keybindings. They can generate metadata to be used by later parsers. They can return effects to happen before or after the keybindings. Effects will no longer be run directly in extension functions but instead returned as lambdas/functions or as quoted list (to support leto.el; see below). Functions are used instead in order to support running effects before or after the keybindings are created.
    • The extension system will be implemented in a forwards-compatible to allow later improvements without breaking existing parsers. In the PoC, the extension parser functions return plists (e.g. keybind parser: (list :keybinds ... :effect ... :after-effect ...)). I'm undecided on whether to pass parser functions keyword arguments or to just recommend &rest _ (since they take few args currently, will likely not take many more, and the keyword arguments are already passed as a plist). Maybe I will use a single plist argument, so that the return value of one parser can be passed directly to the next.
    • Drop support for older Emacs versions and use newer utility functions like when-let and especially pcase-let (with plist destructuring)
  • Performance issues (Optimize general-define-key #180 and general-define-key macro implementation; no need to require general after compile #184) - these will probably matter less and less over time, but there are a couple of fundamental gotchas with general currently that can slow init time
    • Generating prefixes when :global-prefix, :non-normal-prefix, and :prefix are specified is very slow. The current recommendation is to use prefix maps instead. The rewrite could potentially disallow using the :prefix keywords or give a warning (that can be disabled) in situations where it can cause slowdown.
    • General by default will delay keybindings if a keymap doesn't exist (by adding a function to after-load-functions to check if the keymap now exists when loading new elisp). This is very slow. For example, if you create a bunch of evil keybindings before loading evil, loading evil will be very slow. Maybe this can be kept as opt-in to make migration easier, but using with-eval-after-load, use-package's :config, or maybe a newly added :general-config should be recommended instead.
    • This won't be recommended for many reasons (e.g. various caveats like variables needing to be made available at expansion time since macros are not supposed to rely on the values of arguments and also the fact that it probably won't significantly improve startup time), but it will be technically possible to not even depend on any code from the rewrite at load-time by using leto.el and compiling the init file (general-define-key macro implementation; no need to require general after compile #184). Not sure how robust this can be (so far falling back to the normal function definer on void-variable errors seems to work fine), but it will at least be fun to look into further.
  • At this point there are a lot of improvements I want to make that require other breaking changes. The syntax will mostly remain the same (or at least the previous syntax will remain an option).

Other improvements:

  • Documentation rewrite
    • Better introduction/tutorial at beginning
    • Suggested best practices
    • Full reference last
  • Leto.el
    • The vast majority of issues on the general.el repo are questions. General.el causes a lot of confusion for people not yet familiar with Emacs' keybinding system. In addition to having better introductory documentation, it would help if general.el was more transparent. An "instructor" should be provided to expand a definer to human-readable code.
    • This would help users understand how the keybinding system works/what general.el is actually doing
    • This would also help users understand whether an issue they are having is related to general.el (often it isn't)
    • The new extension system can support this with almost no extra code for leto.el (parsers just need to have slightly different behavior depending on whether called from leto or not, e.g. return a quoted form instead of a lambda for an effect)
  • Split into multiple files (load only what you use)
    • Base file will only have the basic key definer
    • Separate file for extra definer macros built on top of the basic key definer
    • Separate file for keybinding helpers
    • Separate file for leto.el
    • Separate file(s) for non-keybinding helpers
    • Separate file for some backwards-compatibility helpers to make migration easier
  • More non-keybinding configuration helpers - General has its own hook, advice, etc. helpers already with syntactic compatibility with add-hook and advice-add and extra functionality (e.g. ability to add multiple functions to multiple hooks and the ability to conditionally remove the function from the hook). I have a long list of ideas for other configuration helpers (either from my own config or yet unwritten) that I think would be better as part of a library. There are also a lot of useful helpers in Doom Emacs, for example, that would be better as part of an external library. I haven't decided if they should be in a separate repo, but I will probably just keep them in a separate file.
  • Other long unimplemented todo items like modifier prefixes (:prefix "C-") and annalist.el integration
  • Like keymap delaying, rethink other things. Make anything that requires guessing (e.g. :general has to guess if the first argument is a definer, general-def has to guess if arguments are keymaps or states, use-package words have to do some weird things for autoloading)
    • Keymaps should probably still be quoted (even though not necessary for delaying, other uses like recording the keymap name to display later with annalist) (like set - can still build setq on top of)
    • Rethink extended definition syntax. If Emacs adds support for a plist definition in the future, the current syntax could be problematic. Even if it doesn't, the current syntax is not ideal. Something like key def :with (extended-def-plist...) (or the equivalent with the plist preceding the definition) could be specially handled instead. I don't really love this non-standard use of keyword argument though and need to think about things more. Maybe this can just be one option and the alternate list for each keybinding syntax below can be recommended.
    • Also consider the best syntax for binding multiple keys at at once to a single command (this can be potentially useful in some cases)
    • Consider alternate syntaxes: (<key> <command> :extended-def-keyword <arg> ...) would be much simpler to parse since the key and command are always in the same format. This will probably be the format for the internal base definer. A let-like (as opposed to setq-like) syntax will probably be optionally provided in a user-facing definer. Not sure what should be the default.
    • Rethink general-def equivalent. What should be the single recommended definer? general-def tried to be close to a drop-in replacement for several other definers, but this means it has some caveats (e.g. the need to use :start-maps or some other bogus keyword here: (general-def keymap :start-maps t key-variable #'command)). Probably it would be better have a definer that supports a variable number of positional arguments but does not try to be a drop-in replacement for other definers in order to avoid these caveats.
    • :prefix, :non-normal-prefix, and :global-prefix interaction is confusing. :prefix should always apply, and there should be :normal-prefix and :non-normal-prefix instead (old behavior can potentially be opt-in).
    • Rethink how :general is handled (the check to determine whether the the first form is a definer or argument is not fully robust).
    • Remove autoload generation functionality? The only case in which this is useful is for functions you have defined yourself (if a package has not correctly defined autoloads, you can still manually use autoload and report it as a bug). For user-defined functions loaded after loading a package, I could just add an :autoload extend and remove all this code.
    • Drop :which-key keyword entirely? which-key-replacement-alist is slow and imperfect but it does provide functionality that the other method does not have. Maybe just recommend the user to manually add these more complex replacements (like those that use a function to determine the string to display).

The rewrite will not be fully compatible with general.el (things deprecated in the readme will finally be deprecated, the extension system will be different, delaying keybindings until the keymap exists will no longer be the default, etc.), so I will probably create a new package to avoid breaking people's configs. I'd like to keep the current name, but that doesn't seem practical. I guess I could create a separate repo and not put it on MELPA, but that would probably end up causing a lot more confusion.

Possible names:

  • tyrant
  • mogul
  • baron
  • consul (previously chosen name, but it would be too confusing now with both consult and counsel)
@noctuid noctuid pinned this issue Sep 29, 2021
@alphapapa
Copy link

Well, I don't understand all of it (like the leto.el parts), but it sounds interesting, and I look forward to it. :)

@tshu-w
Copy link

tshu-w commented Oct 20, 2021

Highly looking forward to it!

@ChauhanT
Copy link

Any chance you might support other modal systems like meow or boon? If the interface is simplified and uniform, maybe these modal systems can also plug into general?

@noctuid
Copy link
Owner Author

noctuid commented Feb 18, 2022

I don't have any plan to directly support other modal systems (e.g. I won't add keymap aliases for them, but any user can in their own configuration). I'd accept a PR for custom definer, but for a lot of them, that doesn't even look necessary. You can already use general with them today. If you have some concrete suggestion to allow for better integration or a question, please open a separate issue.

@noctuid
Copy link
Owner Author

noctuid commented Apr 16, 2022

I've laid out my thoughts on what the new syntax should be here. There will be only one definer used in the documentation for simplicity and flexibility.

General.el has become too much of a monster to easily rewrite all at once. To make this more easily doable, I will initially only implement a minimal feature set: minimal extended definition support (no :which-key, :jump, :repeat, etc.)., no use-package integration, no annalist.el integration, no key definition helpers (like key simulation, key translation, automatic key unbinding, etc.), etc. Eventually, I'll convert my PoC to support the new syntax listed above and then push a minimal version if anyone wants to provide early feedback.

The key definition helpers don't need to be added anytime soon because they can be used standalone from general.el. They will be in a separate package/file when added. I'm not sure yet if I should but them in a separate repo from the definer code.

I am going to spin off all non-keybinding-configuration utilities into a separate package, though they will be left in general for backwards compatibility. This is almost done.

@noctuid
Copy link
Owner Author

noctuid commented May 6, 2022

I've decided to split general.el even up further. I will try to split any novel functionality into separate, standalone packages.

satch.el will have the non-keybinding configuration helpers (settings, hooks, advice, and other delayed evaluation helpers). I've added the initial code/documentation. Don't expect everything to work right until I've added tests (and the missing files), but the documentation is mostly there for anyone interested in understand what utilities will be provided. The big new feature is satch-once, which is a more generic combination of transient hooks, transient advice, and eval-after-load. It allows having named conditions for loading a package or running some code (e.g. first input or file, after first gui frame, after first tty frame, etc.).

;; enable `magit-todos-mode' after loading magit
(use-package magit-todos
  :once "magit")
;; enable `magit-todos-mode' after calling `magit-status'
(use-package magit-todos
  :once #'magit-status)

See here for other practical examples of satch.el use cases taken mostly out of my own config.

Not sure what I will end up using for the names of what general currently has as :ghook and :gfhook. The will be prefixed with :satch- by default, but I am thinking of recommending aliasing them to :hooks and :setup-hooks instead. Not sure. Suggestions welcome.

@noctuid
Copy link
Owner Author

noctuid commented May 6, 2022

Continued:

arcana will be for the key definition utilities (general-key, general-simulate-key, general-key-dispatch, general-predicate-dispatch, etc.). I've often used these to answer people's questions for how to do something more complex, and the response is sometimes "that's great, but I don't use general." Hopefully having these in a smaller library will be useful to those people. I will see if I can get general-chord directly into key-chord where it belongs.

I've added the first commit to familiar. It's not functional because I haven't committed the base definer yet just the code to expand the user-facing syntax to arguments for the base definer. See these examples of the new candidate syntax. Any feedback here would be welcome. The main difference from general-def is how keyword arguments and extended definitions are handled. There is a new way to specify extended definitions, and now you can alter or clear keyword arguments for subsequent definitions. I won't use this much myself, but a lot of people seem to like this syntax (map! and bind-key support it).

Probably in the next few weeks I will remove all the unfinished keywords in the prototype base definer and then commit a barebones working version of familiar that just has the extension system with keymap aliases. It will need some changes, but I think the core should only be a few hundred lines. I will implement evil support next because it touches all four parts of the extension system and will prove (or disprove) that the design is good enough to support the complexity.

Unlike use-package, I am going to try to have a more intelligent way to order transformers instead of having the user try to figure out where the right keyword to add after is (this won't work well since most keywords will be optional). I will try to label various stages and provide a helper function to add a new keyword (e.g. run the function for my newly added keyword after all transformations have been made: my function will only be run for side effects).

Most other keywords (e.g. I will do the prefix keywords next since they need the biggest rework). Most of these will probably stay in the repo as extensions. familiar-which-key will be a separate repo because I don't think it will be necessary for most people and don't what people to mistakenly continue to use :which-key when they should be using which-key's new method (will have builtin :desc keyword for it).

I will also make a separate package for general-override-mode (high precedence keymap and true buffer-local keymap for non-evil users).

@alphapapa
Copy link

alphapapa commented May 7, 2022 via email

@noctuid
Copy link
Owner Author

noctuid commented May 7, 2022

Sorry for the long reply in advance, but hopefully it will also clarify things for anyone else wondering "why do this? is it really necessary" (now and once I release the first version of familiar.el).

FWIW, I feel like I will probably continue using General for a long time. I don't mean to disparage your work or plans, but General works great for me

This is what I expect most people will do and will recommend that anyone who has no issues with general.el continue to do. Nobody wants to overhaul their configuration file every month to switch to the next shiny thing (glad I'm not still primarily using Vim where there are ten thousand implementations of every plugin), and I will probably keep familiar.el "unstable" for a long time so I don't end up locked into poor design decisions like I am for general.el. General.el won't go anywhere, and #503 should go a long way to addressing the major issues people currently have with it before and even after the performance improvements of familiar.el.

That said, I'm not going to make major breaking changes to general.el and break people's setups, and I'm not willing to continue to develop new features on top of the existing code base/syntax without breaking changes. There is a lot of room for improvement for existing functionality, and there are a lot of nice to have minor new features I'd like to add such as more sophisticated keybinding aliases (aliases for multiple keymaps or an alias that uses a function to select keymaps), hydra and hercules integration, the ability to bind a list of keys (which means general-unbind no longer has to be a separate thing), the ability to actually remove a keybinding from a keymap (instead of just binding it to nil), :modifier "C-" "a" #'a-command ... syntax to bind "C-a", annalist.el integration (which is slightly nicer than general-describe-keybindings), etc.

It will be hard to clearly explain until I have a more finished product to show, but I believe that even ignoring the new features mentioned above, familiar.el's other improvements will be compelling enough for a lot of people to eventually switch. There will be a list of value adds over general.el that people can take a look at to decide whether it's worth the switch.

Many of the improvements will be minor, but there will be a lot of them. As soon as satch.el is ready, I plan to immediately switch my own config to using it because :once will simplify/remove a lot of comparably ugly code in my config file.

having to learn how 3 or 4 other packages work and integrate is not generally (ha) something I want to do unless I have to. It's similar to the difference between using Helm and the "SMOCE stack" of completion/selection packages.

I've found the new completion packages compelling, but general.el is not exactly comparable because there is no interaction between general-add-hook and general-def, for example. I can split these and utilities like general-simulate-key into different repos, so that people don't need to install one giant package for what they want. I think this will help a lot of people who would find this functionality useful but aren't aware of it or didn't want to install general.el for it.

If you only use general.el for the key definers and aren't interested in the other functionality, then there will not be much new to learn. There will be only a single comparable key definer package that provides familiar syntax to general-def (one of the reasons it's named familiar.el; I wanted another name with a dual meaning). There will also be a migration guide. A lot of general-def calls will be a simple search and replace, and others will not be that hard to change. I have nearly 800 uses of (general-...) and general use-package keywords in my own config, and I think for me (and hopefully anyone with the migration guide), it won't take that much time to change all of them. The main difference from general.el is that most functionality will be optional. The practical impact will mostly be a matter of having to install additional packages (e.g. if you want evil support) rather than having to do extra integration/configuration. Something like (require 'familiar-defaults) will allow getting a sane feature set comparable to general-def.

If you use general.el for non-key-definer functionality, then transitioning for those functionalities will even more so mostly just be search/replace.

it took some time to learn and configure them, while Helm was already doing everything I needed well

I think it's currently much harder to learn to use general.el because of how large and disconnected the feature set is than it would be if it was multiple, smaller packages. Helm is at least split into multiple files. For someone who already knows general.el that doesn't matter, but even a lot of existing users don't really understand it or know what functionality it provides. Users frequently post questions about general.el here, on the emacs subreddit, etc. A shorter, more focused manual and a more transparent definer with a helper to generate user-readable code via macro expansions to show what familiar.el is actually doing will help a lot.

I think the value add of familiar.el will be massive for confused users, newcomers, and people who previously dismissed general.el for various reasons (e.g. "I don't need evil support" or "too big"). The benefits for me as the developer of the package in maintaining/extending the package will be similarly significant. While the benefits for me as a user of it may be comparably minor, I'm still doing it because:

  1. It's an enjoyable problem for me to tackle.
  2. People seem to find this package useful, but I can't see the implementation lasting for decades. I don't enjoy working with it now, and I wouldn't want to leave this mess to someone else. I think they would eventually conclude the same thing as me: the user-facing side is a nice start, but it needs to be rewritten.
  3. A main focus of these configuration utility packages like general, use-package, and alternatives for me is to simplify configuring Emacs, and I think even minor improvements here are important. I think the success of Doom and other starter kits proves that reducing the barrier to entry can bring new users who would have otherwise not tried Emacs. For familiar.el, I'd rather provide something that is not unnecessarily confusing and overwhelming in order to help new users or users who want to move to using their own configuration. For satch.el, I'm trying to provide equivalents a lot of the utilities that people find useful in Doom for those that don't use Doom.

existing solutions have a lot of value, and rewriting working software is always a risk/reward scenario.

Agreed. In this case though, the most useful code does not need to be rewritten (e.g. satchel.el is not a rewrite, and most of the other packages will not be complete rewrites either). Only the core of general-define-key needs a rewrite. It's rigid, and if, for example, a an equally complex evil alternative appeared (in terms of keymap handling), it would be a mess for general.el to try to support it.

The only value of the current general-define-key implementation is the knowledge I've gained from writing it. The code/extension system works but is a fundamentally flawed dead-end. From a development perspective there are also a some barely used features that are arguably bad ideas I want to drop support for because they add so much complexity.

I'm currently investigating the possibility of building a lispy DSL for key definition that allows using arbitrary forms (e.g. and, let, etc.) on top of which everything else will be built. This sounds complicated or over-engineered, but it won't be like cl-loop. It will mostly just be lisp with user-definable local macros. This is inspired by setup.el whose extension system is <300 LoC and simultaneously incredibly powerful and easier to customize and rationalize about than use-package's extension system (oh man how I hate the code in general to mimic how use-packge automatically creates autoloads and how some keywords can sometimes imply :defer t; both barely useful features). This key definition DSL should be much simpler to implement than all the intertwined/hardcoded functionality in general.el, allow the extension system to be much simpler to use, allow much more easily generating human-readable expansions, and be infinitely more flexible. For example general-translate-key is impossible to implement using general-define-key alone (there's an extra definer on top of it), but it could be easily done directly with this DSL. It will also be usable for anyone who hates general.el syntax and wants a base on which to implement an alternate definer/syntax without much effort (e.g. bind-key syntax; it should no longer be necessary to develop new key definers from scratch). Will this be generally useful for users? Not for basic keybindings, but it will allow doing what doom's map! does with conditional keybindings and a lot of other advanced functionality without me having to write any additional code to support it.

Still, even this won't be a rewrite from scratch because I can mostly reuse a lot of the existing code for keywords and just alter it slightly to plug it into the new extension system.

If almost no existing users switch in the first 5 or more years after the initial release, that's fine with me. The goal is more long term:

  1. To try to write something general enough to still be usable without a major rewrite regardless of whatever new packages and functionality come out in the next couple of decades
  2. To write something that will be simpler to maintain/develop in the case that I lose interest and no longer want to maintain the project
  3. To write an underlying extension system that is flexible enough so that if I later again decide "well this user-facing syntax was actually a bad idea," I don't have to change the vast majority of the code and can just write a few hundred lines of macros instead of a complete rewrite. Someone who preferred the old syntax could stick with it and still get any new functionality the underlying DSL added. Basically, I want to create a library for creating key definers.

I think a rewrite of the core is inevitable. The goal is to do it in such a way so that another rewrite in next the 10+ years won't be.

@alphapapa
Copy link

alphapapa commented May 8, 2022 via email

@ChauhanT
Copy link

Hi, I'm trying to figure out how to implement some sort of modal bindings for meow using a modal leader-key. If I use (general-create-definer map-modeleader ...) etc., it does not work - because I want there to be two or more keymaps concurrently active as a pre-condition for the bindings to work. E.g., for a binding for prog-mode I want the prog-mode-map as well as the meow-motion-state-keymap. Currently, the :keymaps vector is parsed as an 'or' operation rather than 'and'. Would you have any recommendations as to how I can accomplish this?

@noctuid
Copy link
Owner Author

noctuid commented Mar 15, 2023

Emacs keybindings can't support that. Keys always end up being bound in a single keymap. In the case of evil you can have something like 'normal prog-mode-map as the user-facing syntax, but this translates to a single keymap. Does meow support this defining something like 'normal prog-mode-map? Looking at meow-helpers I only saw a key definer that binds directly in insert, motion, etc. If meow supports it, point me to the key definer used and I can tell you how you would do it with general (please open a new issue in this case).

@ChauhanT
Copy link

Thanks for taking the time to explain the issue so well. I am not sure if meow allows for these maps - I'll certainly look into it. I am the dev/maintainer of emacs-groundup, and I have been contemplating a move away from the evil ecosystem and into meow. I was half way through the transition and realised I couldn't do mode-based leader keys =.="

When you say 'normal prog-mode-map , you mean 'normal-prog-mode-map ?

@noctuid
Copy link
Owner Author

noctuid commented Mar 15, 2023

I mean the syntax (evil-define-key 'normal prog-mode-map ...) translates the state and parent keymap into a single "auxiliary" keymap to make a keybinding in (see evil-get-auxiliary-keymap). The implementation of evil's keymap system is somewhat complex; meow may not support this sort of thing.

@ChauhanT
Copy link

Gotcha. Yeah, you might be right. After all evil is a very comprehensive and meticulously engineered package. meow, instead, seeks to offer a much simpler set of modal options; which is why I a drawn to it - it leaves emacs feeling like emacs, and not vim.

I'll explore this further and report back. And I'll also try to ask the developer of meow if they have any idea/recommendations on how to do this.

@ChauhanT
Copy link

Hi, coming back after a couple of weekends of exploration. The author of meow directed me to this particular thread and message : meow-edit/meow#126 (comment).

I have been doing some testing using a config like this:

(use-package general
  :straight t
  :config
  (general-override-mode 1)
  (general-auto-unbind-keys)

  ;; We need the leader-key commands to replace the limited menus from meow-keypad
  (general-create-definer +m/map-leader
    :keymaps '(meow-motion-state-keymap meow-normal-state-keymap)
    :prefix "SPC")

  ;; Create a "mode-leader"
  (general-create-definer +m/map-modeleader
    :prefix "SPC SPC"
    "" '(:ignore t :wk "modebindings")))

My mode-specific mappings end up covering all state, including the insert state.

I have tried to make a function which will take a mode and a list of keybindings as inputs, and will then add a hook to the said mode which only maps to meow-normal-state-keymap. This also fails miserably. However, in this case, I think it might be my lack of elisp skills... Any tips or comments will be very helpful !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants