Skip to content

modules/keymaps: fix lua option deprecation warning#3960

Merged
GaetanLepage merged 1 commit intonix-community:mainfrom
MattSturgeon:main
Nov 21, 2025
Merged

modules/keymaps: fix lua option deprecation warning#3960
GaetanLepage merged 1 commit intonix-community:mainfrom
MattSturgeon:main

Conversation

@MattSturgeon
Copy link
Member

@MattSturgeon MattSturgeon commented Nov 21, 2025

The deprecation warning for the keymap-submocule lua option relied on getSubOptions, however this is fundamentally flawed because that function uses a different module eval from the one that merges submodule definitions.

Since definitions are not used by getSubOptions, options.lua.isDefined will never be true.

Instead, we have two choices:

  1. Add a luaIsDefined option to the keymap submodule
  2. Use the new v2 merge's valueMeta to access the actual module eval

In this PR I've implemented option 2, and added tests to ensure the warning actually works. Actually testing that warnings and deprecated options work as expected is important, as demonstrated by this one being broken so long without us noticing.

Since the warning will not have been shown to any users, I've pushed back the "will be removed after" epoch. (Sadly, I started working on this because I was hoping to delete all the deprecated lua option code... Then I noticed it was fundamentally flawed).

The deprecation warning for the keymap-submocule `lua` option relied on
`getSubOptions`, however this is fundamentally flawed because that
function returns uses a different module eval from the one that merges
submodule definitions.

Since definitions are not used by `getSubOptions`,
`options.lua.isDefined` will never be true.

Instead, we have two choices:

1. Add a `luaIsDefined` option to the keymap submodule
2. Use the new v2 merge's `valueMeta` to access the actual module eval
Copy link
Member

@GaetanLepage GaetanLepage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!!!

@GaetanLepage GaetanLepage added this pull request to the merge queue Nov 21, 2025
Merged via the queue into nix-community:main with commit d0b0b75 Nov 21, 2025
4 checks passed
@VvSsNPk
Copy link

VvSsNPk commented Nov 21, 2025

… while evaluating the option system.build.toplevel':

   … while evaluating definitions from `/nix/store/hr5pl7m833kkr0zv98iydy7mxyl10qld-source/nixos/modules/system/activation/top-level.nix':

   … while evaluating the option `warnings':

   … while evaluating definitions from `/nix/store/a4la088hi5id7sw77gy9zdvwn83dgczz-source/nixos/common.nix':

   … while evaluating the option `home-manager.users.royaleinstein.warnings':

   … while evaluating definitions from `/nix/store/ykn2jzkrv3k7zcafjkyddyj31bd6f9bx-source/wrappers/_shared.nix':

   … while evaluating the option `home-manager.users.royaleinstein.programs.nixvim.warnings':

   … while evaluating definitions from `/nix/store/ykn2jzkrv3k7zcafjkyddyj31bd6f9bx-source/modules/keymaps.nix':

   (stack trace truncated; use '--show-trace' to show the full, detailed trace)

   error: attribute 'valueMeta' missing
   at /nix/store/ykn2jzkrv3k7zcafjkyddyj31bd6f9bx-source/modules/keymaps.nix:61:15:
       60|             [
       61|               options.keymaps.valueMeta.list
         |               ^
       62|               (lib.concatMap (ev: ev.list) (lib.attrValues options.keymapsOnEvents.valueMeta.attrs))

~`

Getting this error when updating nixvim, feel like this error has something to do with this change

@MattSturgeon
Copy link
Member Author

MattSturgeon commented Nov 21, 2025

Thanks for reporting!

Do you have a really old nixpkgs revision somewhere?

If lib is being constructed from a nixpkgs revision that doesn't have NixOS/nixpkgs#391544, then this error is expected.

We can handle that edge-case better, though 🤔

@AleAndForCode
Copy link

Thanks for reporting!

Do you have a really old nixpkgs revision somewhere?

If lib is being constructed from a nixpkgs revision that doesn't have NixOS/nixpkgs#391544, then this error is expected.

We can handle that edge-case better, though 🤔

I've just updated my flake inputs and got the same error for latest both stable and unstable nixpkgs

@SJ-Fang23
Copy link

Same here, In my case, Nixvim is following NixOS-Unstable branch of the NixPkgs. Maybe the ValueMeta update hasn't made it way into downstream NixOS-Unstable branch?

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

Successfully merging this pull request may close these issues.

5 participants