Allow disabling nvf mappings

[alfarelcynthesis]

This approach to the problem sources mkMappingOption from config, since it should reference an option for whether to use the provided default or null.

This change would be completely transparent to any current users, but requires enforcing that new plugin contributions use config.vim.lib.mkMappingOption instead of the lib.nvim.binds version.
If people are using the original mkMappingOption in outside projects, it will continue working as before, which is probably the desired behaviour.

• First commit contains the actual implementation.
  • Probably best to focus review time here initially, it's fairly simple.
• Second is mostly script-generated, replaces references to the original mkMappingOption with ones to the config-aware one.
• Third is fixes to less-automatable issues related to nullable keymaps.
• Fourth is conversions to mkMappingOption for options that should have used it but used to use mkOption.

Related discussion: #1043

Feedback Wanted

• Should the lib options go elsewhere? be named something else?
• Should the config-aware mkMappingOption be named separately to reduce confusion?
• Should useNvfKeymaps be named something else?
• Should this be mentioned in the setup/other docs?
• Should the option also be used to remove which-key group defaults (Duplicate Keybindings when setting personal keymaps in which-key #718)?
  • Probably not, based on the comment on the issue.

Remaining work

• Direct testing. Packages build, but I should spend some more time inspecting them in an actual setup.
• Make surround-nvim vendored keymaps option default depend on the global setting?
• LS keybinds, does rust still require them to be not null?
• Should the copilot-lua mkLuaKeymap use lib.binds.mkLuaBinding?
  • Updated it to allow nulls, should be fine to leave as is, but might be a good opportunity to update it?
• Check that .#nix and .#maximal build with useNvfKeymaps disabled.

Sanity Checking

• I have updated the changelog as per my changes
• I have tested, and self-reviewed my code
  • I haven't tested every plugin, but the changes should be completely transparent, and I've tested both packages with and without the option enabled, as well as my config.
    This is probably worth testing a bit more thoroughly, if it gets accepted.
• My changes fit guidelines found in hacking nvf
• Style and consistency
  • I ran Alejandra to format my code (nix fmt)
  • My code conforms to the editorconfig configuration of the project
  • My changes are consistent with the rest of the codebase
    • debatable, worth talking about (function as config option)
• If new changes are particularly complex:
  • My code includes comments in particularly complex areas
  • I have added a section in the manual
  • (For breaking changes) I have included a migration guide
    • isn't breaking
• Package(s) built:
  • .#nix (default package)
  • .#maximal
  • .#docs-html (manual, must build)
  • .#docs-linkcheck (optional, please build if adding links)
    • unrelated errors
• Tested on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin

---

Add a 👍 reaction to pull requests you find important.

[github-actions]

🚀 Live preview deployed from b4d6539

View it here:

Debug Information

Triggered by: alfarelcynthesis

HEAD at: no-default-mappings-option

Reruns: 2425

[horriblename]

Thanks! I wouldn't have thought of that

LGTM but I'd like an approval from @NotAShelf or @Soliprem

[snoweuph]

Thanks! I wouldn't have thought of that

LGTM but I'd like an approval from @NotAShelf or @Soliprem

isn't raf on for vacation according to matrix?

[NotAShelf]

I'm on """vacation""", meaning I am away from my main system involuntarily

[snoweuph]

I'm on """vacation""", meaning I am away from my main system involuntarily

LMFAO, :3

[alfarelcynthesis]

I added the changelog entry, seems like this won't need too many other changes :)

[alfarelcynthesis]

Rebased on latest main + #1457.

(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.)

[Soliprem]

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

[NotAShelf]

This is an interesting solution. I'm quite positive that this is the only way of scoping config and lib together, but I'm also not a huge fan of putting lib inside config. That said, I also cannot think of a better alternative so I'm not going to block this any further.

One thing I want to note is that useNvfKeymaps could probably be renamed to something like vim.keymaps.builtin.enable. I like this more with the possibility of renaming this project (if ever) and the possibility of adding more knobs for builtin keymaps in mind. Also open to different names idea if you think vim.keymaps.builtin should not be reserved like that.

Left one comment, but it's not a blocker. You can get that while renaming the option if you'd like, but not critical. I'd also like to mention this in the rendered manual, a quick entry (as a note/important level admonition) would be perfectly okay in the "Custom keymaps" section I think.

Thank you for your good work, and your patience so far.

[horriblename]

One thing I want to note is that useNvfKeymaps could probably be renamed to something like vim.keymaps.builtin.enable

builtin is a bit ambiguous imo - builtin to neovim or builtin to nvf? vendored is a bit better, but also open to better names

[alfarelcynthesis]

Unfortunately, since vim.keymaps is a list type, I can't add options there directly. Maybe vim.vendoredKeymaps is better? I don't think adding to the legacy maps is a good idea.

Honestly, all the nvf-specific one-off options might be better placed in a specific attrset so there aren't conflicts like this, but that would be annoying migration given most of them work fine as they are.

For now, I just updated the option to remove nvf from the name, but I'm of course up to change it if there's ideas.

[alfarelcynthesis]

This is an interesting solution. I'm quite positive that this is the only way of scoping config and lib together, but I'm also not a huge fan of putting lib inside config. That said, I also cannot think of a better alternative so I'm not going to block this any further.

One thing I want to note is that useNvfKeymaps could probably be renamed to something like vim.keymaps.builtin.enable. I like this more with the possibility of renaming this project (if ever) and the possibility of adding more knobs for builtin keymaps in mind. Also open to different names idea if you think vim.keymaps.builtin should not be reserved like that.

Left one comment, but it's not a blocker. You can get that while renaming the option if you'd like, but not critical. I'd also like to mention this in the rendered manual, a quick entry (as a note/important level admonition) would be perfectly okay in the "Custom keymaps" section I think.

Thank you for your good work, and your patience so far.

You mention a (presumably separate) comment here but I can't seem to find it. Do you remember what it was that you preferred changed (other than the option name and a doc hint)?

[alfarelcynthesis]

Also, I've added a couple lines to the Custom keymaps section, but looking at the rendered docs I can't actually find the section.