This repository was archived by the owner on Aug 23, 2025. It is now read-only.
Allow multiple keymaps in :map argument#1051
Open
fishyfriend wants to merge 2 commits intojwiegley:masterfrom
Open
Allow multiple keymaps in :map argument#1051fishyfriend wants to merge 2 commits intojwiegley:masterfrom
fishyfriend wants to merge 2 commits intojwiegley:masterfrom
Conversation
This updates the bind-keys functions to accept either a symbol or a
list as argument for the `:map' keyword, with additional related
fixes:
(1) Handle the keymap name `nil' as a synonym for `global-map';
(2) Fail if an invalid argument is specified for `:prefix-map' or
`:repeat-map' keywords.
|
@jwiegley Would it be possible to merge this pull request? It seems like you were ready to merge the previous version and the only obstacle was the copyright assignment legal requirement, which is now addressed. Thanks. |
Contributor
Author
|
Hi, original submitter here, responding to @benthamite's comment. IMO a fresh review is needed before merging. Although this PR is not a huge set of changes, the part of bind-key.el involved is pretty complex, the functional changes are not totally identical to the original PR (see description), and the revised PR touches code that moved after the original version was reverted. |
|
@fishyfriend, oh, I see. Thanks for clarifying, and please disregard my previous comment. |
This was referenced Sep 17, 2024
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a reimplementation of #830, released as #1019 but reverted due to lacking copyright assignment. The copyright issue should now be solved as I have signed the Emacs copyright papers.
Sorry for the trouble of requesting a second review. I felt a rewrite was warranted due to discovered loose ends as well as readability.
Noteworthy changes:
:map nil, which currently causes an error, is now treated as synonymous with:map global-map.nil,global-map, andoverride-global-mapare explicitly disallowed as arguments for:prefix-mapor:repeat-map. Existing checks here and here may have been intended for this purpose (perhaps mistakenly substitutingmapfor(cadr args)?). These checks had simply been removed in the original PR as I couldn't make sense of them.A redundant check related to repeat maps has been removed
Original description from #830
This PR augments bind-keys to support passing a list of keymaps as the
:mapargument.When one wants to bind the same key/command pair in multiple keymaps, the current options have some drawbacks:
With the new way, it is possible to avoid duplication and still use
:bind:Additionally this PR clarifies the documentation of
bind-chordsto reflect that this style is already available for that command.Thanks,
Jacob