Allow disabling nvf mappings#1450
Open
alfarelcynthesis wants to merge 5 commits intoNotAShelf:mainfrom
Open
Conversation
alfarelcynthesis
force-pushed
the
no-default-mappings-option
branch
2 times, most recently
from
7f2ed04 to
7823de1
Compare
alfarelcynthesis
force-pushed
the
no-default-mappings-option
branch
5 times, most recently
from
781252c to
3a40efb
Compare
alfarelcynthesis
force-pushed
the
no-default-mappings-option
branch
from
3a40efb to
0f14749
Compare
alfarelcynthesis
requested review from
NotAShelf,
Soliprem and
horriblename
as code owners
horriblename
previously approved these changes
Mar 15, 2026
Collaborator
horriblename
left a comment
horriblename
left a comment
There was a problem hiding this comment.
Thanks! I wouldn't have thought of that
LGTM but I'd like an approval from @NotAShelf or @Soliprem
Collaborator
isn't raf on for vacation according to matrix? |
Owner
|
I'm on """vacation""", meaning I am away from my main system involuntarily |
Collaborator
LMFAO, :3 |
Contributor
Author
|
I added the changelog entry, seems like this won't need too many other changes :) |
alfarelcynthesis
force-pushed
the
no-default-mappings-option
branch
from
cbddc5c to
0ba2482
Compare
alfarelcynthesis
force-pushed
the
no-default-mappings-option
branch
from
0ba2482 to
c5c9ef8
Compare
Contributor
Author
|
Rebased on latest main (I'm rebasing to make sure to catch small incompatibilities as they get into the repo, given this touches a lot of files.) (Also means anyone testing this branch by using it gets up-to-date options elsewhere.) |
alfarelcynthesis
force-pushed
the
no-default-mappings-option
branch
from
c5c9ef8 to
40f40a4
Compare
alfarelcynthesis
force-pushed
the
no-default-mappings-option
branch
from
40f40a4 to
a62ed88
Compare
alfarelcynthesis
force-pushed
the
no-default-mappings-option
branch
2 times, most recently
from
d48066a to
997c941
Compare
alfarelcynthesis
force-pushed
the
no-default-mappings-option
branch
from
997c941 to
78a4db0
Compare
alfarelcynthesis
force-pushed
the
no-default-mappings-option
branch
from
78a4db0 to
5f51010
Compare
Soliprem
reviewed
Mar 25, 2026
Collaborator
Soliprem
left a comment
Soliprem
left a comment
There was a problem hiding this comment.
on a skim nothing jumps to the eye that I'd change, and I like the change itslef. LGTM, but I'll need to review this better with a fresh mind
mkMappingOption wants to read a setting set in config without passing it every time, so it should be defined in config.
Generated using:
- `sd -F "inherit (lib.nvim.binds) mkMappingOption;" "inherit (config.vim.lib) mkMappingOption;" $(find . -type f)`
- `sd -F "{lib, ...}: let" "{config, lib, ...}: let" $(find . -type f)`
Tweaked manually (placement in inherit list, fixing todo-comments and toggleterm).
Ran `nix run nixpkgs#deadnix -- -e` to clean up.
Next commit includes unrelated dead code.
Mostly involves filtering keybinds that are null in cases where the keybind is the attrname, and optionalString for manual lua keybind registration.
alfarelcynthesis
force-pushed
the
no-default-mappings-option
branch
from
5f51010 to
5ba3b72
Compare
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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 approach to the problem sources
mkMappingOptionfromconfig, since it should reference an option for whether to use the provided default ornull.This change would be completely transparent to any current users, but requires enforcing that new plugin contributions use
config.vim.lib.mkMappingOptioninstead of thelib.nvim.bindsversion.If people are using the original
mkMappingOptionin outside projects, it will continue working as before, which is probably the desired behaviour.mkMappingOptionwith ones to the config-aware one.mkMappingOptionfor options that should have used it but used to usemkOption.Related discussion: #1043
Feedback Wanted
mkMappingOptionbe named separately to reduce confusion?useNvfKeymapsbe named something else?Remaining work
surround-nvimvendored keymaps option default depend on the global setting?copilot-luamkLuaKeymapuselib.binds.mkLuaBinding?.#nixand.#maximalbuild withuseNvfKeymapsdisabled.Sanity Checking
This is probably worth testing a bit more thoroughly, if it gets accepted.
nix fmt).#nix(default package).#maximal.#docs-html(manual, must build).#docs-linkcheck(optional, please build if adding links)x86_64-linuxaarch64-linuxx86_64-darwinaarch64-darwinAdd a 👍 reaction to pull requests you find important.