lib/*-plugin: use lib.mkPackageOption internally

Instead of maintainers providing an actual `defaultPackage`, they should specify the pkg name which we'll use when calling `lib.mkPackageOption`. This makes `mkVimPlugin` and `mkNeovimPlugin` compliant with nix-community#1950.
traxys · Sep 4, 2024 · 285f6cb · 285f6cb
1 parent c8bb788
commit 285f6cb
Show file tree

Hide file tree

Showing 6 changed files with 150 additions and 109 deletions.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -44,7 +44,8 @@ The vast majority of plugins fall into one of those two categories:
   - `name`: The name of the plugin. The resulting nixvim module will have `plugins.<name>` as a path.\
     For a plugin named `foo-bar.nvim`, set this to `foo-bar` (subject to exceptions).
   - `originalName`: The "real" name of the plugin (i.e. `foo-bar.nvim`). This is used mostly in documentation.
-  - `defaultPackage`: The nixpkgs package for this plugin (e.g. `pkgs.vimPlugins.foo-bar-nvim`).
+  - `package`: The nixpkgs package attr for this plugin
+     e.g. `"foo-bar-nvim` for `pkgs.vimPlugins.foo-bar-nvim`, or `[ "hello" "world" ]` for `pkgs.hello.world`.
   - `maintainers`: Register yourself as a maintainer for this plugin:
     - `[lib.maintainers.JosephFourier]` if you are already registered as a [`nixpkgs` maintainer](https://github.com/NixOS/nixpkgs/blob/master/maintainers/maintainer-list.nix)
     - `[helpers.maintainers.GaspardMonge]` otherwise. (Also add yourself to [`maintainers.nix`](lib/maintainers.nix))

diff --git a/docs/user-guide/faq.md b/docs/user-guide/faq.md
@@ -29,6 +29,8 @@ The [nixpkgs manual](https://nixos.org/manual/nixpkgs/stable/#managing-plugins-w
 
 When using NixVim it is possible to encounter an error of the type `attribute 'name' missing`, for example it could look like:
 
+<!-- TODO: Update example now that we use `mkPackageOption` -->
+
 ```
        (stack trace truncated; use '--show-trace' to show the full trace)
 

diff --git a/lib/neovim-plugin.nix b/lib/neovim-plugin.nix
@@ -17,7 +17,7 @@ with lib;
     {
       name,
       maintainers,
-      url ? defaultPackage.meta.homepage,
+      url ? throw "default",
       imports ? [ ],
       description ? null,
       # deprecations
@@ -28,7 +28,13 @@ with lib;
       colorscheme ? name,
       # options
       originalName ? name,
-      defaultPackage,
+      # WARNING: `defaultPackage` is deprecated by `package`,
+      defaultPackage ? throw "mkVimPlugin called without either `package` or `defaultPackage`.",
+      # Can be a string, a list of strings, or a module option:
+      # - A string will be intrpreted as `pkgs.vimPlugins.${package}`
+      # - A list will be interpreted as a "pkgs path", e.g. `pkgs.${elem1}.${elem2}.${etc...}`
+      # - An option will be used as-is, but should be built using `lib.mkPackageOption`
+      package ? helpers.mkPluginPackageOption originalName defaultPackage,
       settingsOptions ? { },
       settingsExample ? null,
       settingsDescription ? "Options provided to the `require('${luaName}')${setup}` function.",
@@ -42,22 +48,78 @@ with lib;
       extraPackages ? [ ],
       callSetup ? true,
       installPackage ? true,
-    }:
+    }@args:
     let
       namespace = if isColorscheme then "colorschemes" else "plugins";
+
+      module =
+        {
+          config,
+          options,
+          pkgs,
+          ...
+        }:
+        let
+          cfg = config.${namespace}.${name};
+          opt = options.${namespace}.${name};
+          extraConfigNamespace = if isColorscheme then "extraConfigLuaPre" else "extraConfigLua";
+        in
+        {
+          meta = {
+            inherit maintainers;
+            nixvimInfo = {
+              inherit description;
+              url = args.url or opt.package.default.meta.homepage;
+              path = [
+                namespace
+                name
+              ];
+            };
+          };
+
+          options.${namespace}.${name} =
+            {
+              enable = mkEnableOption originalName;
+              package =
+                if lib.isOption package then
+                  package
+                else
+                  lib.mkPackageOption pkgs originalName {
+                    default =
+                      if builtins.isList package then
+                        package
+                      else
+                        [
+                          "vimPlugins"
+                          package
+                        ];
+                  };
+            }
+            // optionalAttrs hasSettings {
+              settings = helpers.mkSettingsOption {
+                description = settingsDescription;
+                options = settingsOptions;
+                example = settingsExample;
+              };
+            }
+            // extraOptions;
+
+          config = mkIf cfg.enable (mkMerge [
+            {
+              extraPlugins = (optional installPackage cfg.package) ++ extraPlugins;
+              inherit extraPackages;
+            }
+            (optionalAttrs callSetup {
+              ${extraConfigNamespace} = ''
+                require('${luaName}')${setup}(${optionalString (cfg ? settings) (helpers.toLuaObject cfg.settings)})
+              '';
+            })
+            (optionalAttrs (isColorscheme && (colorscheme != null)) { colorscheme = mkDefault colorscheme; })
+            (extraConfig cfg)
+          ]);
+        };
     in
     {
-      meta = {
-        inherit maintainers;
-        nixvimInfo = {
-          inherit description url;
-          path = [
-            namespace
-            name
-          ];
-        };
-      };
-
       imports =
         let
           basePluginPath = [
@@ -67,49 +129,10 @@ with lib;
           settingsPath = basePluginPath ++ [ "settings" ];
         in
         imports
+        ++ [ module ]
         ++ (optional deprecateExtraOptions (
           mkRenamedOptionModule (basePluginPath ++ [ "extraOptions" ]) settingsPath
         ))
-        ++ (nixvim.mkSettingsRenamedOptionModules basePluginPath settingsPath optionsRenamedToSettings)
-        ++ [
-          (
-            { config, ... }:
-            {
-              config =
-                let
-                  cfg = config.${namespace}.${name};
-                  extraConfigNamespace = if isColorscheme then "extraConfigLuaPre" else "extraConfigLua";
-                in
-                mkIf cfg.enable (mkMerge [
-                  {
-                    extraPlugins = (optional installPackage cfg.package) ++ extraPlugins;
-                    inherit extraPackages;
-                  }
-                  (optionalAttrs callSetup {
-                    ${extraConfigNamespace} = ''
-                      require('${luaName}')${setup}(${optionalString (cfg ? settings) (helpers.toLuaObject cfg.settings)})
-                    '';
-                  })
-                  (optionalAttrs (isColorscheme && (colorscheme != null)) { colorscheme = mkDefault colorscheme; })
-                  (extraConfig cfg)
-                ]);
-            }
-          )
-        ];
-
-      options.${namespace}.${name} =
-        {
-          enable = mkEnableOption originalName;
-
-          package = helpers.mkPluginPackageOption originalName defaultPackage;
-        }
-        // optionalAttrs hasSettings {
-          settings = helpers.mkSettingsOption {
-            description = settingsDescription;
-            options = settingsOptions;
-            example = settingsExample;
-          };
-        }
-        // extraOptions;
+        ++ (nixvim.mkSettingsRenamedOptionModules basePluginPath settingsPath optionsRenamedToSettings);
     };
 }
diff --git a/lib/options.nix b/lib/options.nix
@@ -308,6 +308,7 @@ rec {
         );
     };
 
+  # TODO: Deprecated 2024-09-02; remove once all internal uses are gone
   mkPackageOption =
     args:
     # A default package is required
@@ -326,6 +327,7 @@ rec {
       }
     );
 
+  # TODO: Deprecated 2024-09-02; remove once all internal uses are gone
   mkPluginPackageOption =
     name: default:
     mkOption {

diff --git a/lib/vim-plugin.nix b/lib/vim-plugin.nix
@@ -4,7 +4,7 @@ with lib;
   mkVimPlugin =
     {
       name,
-      url ? if defaultPackage != null then defaultPackage.meta.homepage else null,
+      url ? throw "default",
       maintainers,
       imports ? [ ],
       description ? null,
@@ -16,7 +16,13 @@ with lib;
       colorscheme ? name,
       # options
       originalName ? name,
-      defaultPackage ? null,
+      # WARNING: `defaultPackage` is deprecated by `package`,
+      defaultPackage ? throw "mkVimPlugin called without either `package` or `defaultPackage`.",
+      # Can be a string, a list of strings, or a module option:
+      # - A string will be intrpreted as `pkgs.vimPlugins.${package}`
+      # - A list will be interpreted as a "pkgs path", e.g. `pkgs.${elem1}.${elem2}.${etc...}`
+      # - An option will be used as-is, but should be built using `lib.mkPackageOption`
+      package ? helpers.mkPluginPackageOption originalName defaultPackage,
       settingsOptions ? { },
       settingsExample ? null,
       globalPrefix ? "",
@@ -25,17 +31,10 @@ with lib;
       extraConfig ? cfg: { },
       extraPlugins ? [ ],
       extraPackages ? [ ],
-    }:
+    }@args:
     let
       namespace = if isColorscheme then "colorschemes" else "plugins";
 
-      # does this evaluate package?
-      packageOption =
-        if defaultPackage == null then
-          { }
-        else
-          { package = helpers.mkPluginPackageOption name defaultPackage; };
-
       createSettingsOption = (isString globalPrefix) && (globalPrefix != "");
 
       settingsOption = optionalAttrs createSettingsOption {
@@ -54,42 +53,61 @@ with lib;
         };
       };
 
-      modules = [
-        (
-          { config, ... }:
-          let
-            cfg = config.${namespace}.${name};
-          in
-          {
-            config = mkIf cfg.enable (mkMerge [
-              {
-                inherit extraPackages;
-                globals = mapAttrs' (n: nameValuePair (globalPrefix + n)) (cfg.settings or { });
-                # does this evaluate package? it would not be desired to evaluate package if we use another package.
-                extraPlugins = extraPlugins ++ optional (defaultPackage != null) cfg.package;
-              }
-              (optionalAttrs (isColorscheme && (colorscheme != null)) { colorscheme = mkDefault colorscheme; })
-              (extraConfig cfg)
-            ]);
-          }
-        )
-      ];
+      module =
+        {
+          config,
+          options,
+          pkgs,
+          ...
+        }:
+        let
+          cfg = config.${namespace}.${name};
+          opt = options.${namespace}.${name};
+        in
+        {
+          meta = {
+            inherit maintainers;
+            nixvimInfo = {
+              inherit description;
+              url = args.url or opt.package.default.meta.homepage;
+              path = [
+                namespace
+                name
+              ];
+            };
+          };
+
+          options.${namespace}.${name} = {
+            enable = mkEnableOption originalName;
+            package =
+              if lib.isOption package then
+                package
+              else
+                lib.mkPackageOption pkgs originalName {
+                  default =
+                    if builtins.isList package then
+                      package
+                    else
+                      [
+                        "vimPlugins"
+                        package
+                      ];
+                };
+          } // settingsOption // extraOptions;
+
+          config = mkIf cfg.enable (mkMerge [
+            {
+              inherit extraPackages;
+              globals = mapAttrs' (n: nameValuePair (globalPrefix + n)) (cfg.settings or { });
+              # does this evaluate package? it would not be desired to evaluate package if we use another package.
+              extraPlugins = extraPlugins ++ optional (cfg.package != null) cfg.package;
+            }
+            (optionalAttrs (isColorscheme && (colorscheme != null)) { colorscheme = mkDefault colorscheme; })
+            (extraConfig cfg)
+          ]);
+        };
     in
     {
-      meta = {
-        inherit maintainers;
-        nixvimInfo = {
-          inherit description url;
-          path = [
-            namespace
-            name
-          ];
-        };
-      };
-      options.${namespace}.${name} = {
-        enable = mkEnableOption originalName;
-      } // settingsOption // packageOption // extraOptions;
-
       imports =
         let
           basePluginPath = [
@@ -99,11 +117,10 @@ with lib;
           settingsPath = basePluginPath ++ [ "settings" ];
         in
         imports
+        ++ [ module ]
         ++ (optional (deprecateExtraConfig && createSettingsOption) (
           mkRenamedOptionModule (basePluginPath ++ [ "extraConfig" ]) settingsPath
         ))
-        ++ (nixvim.mkSettingsRenamedOptionModules basePluginPath settingsPath optionsRenamedToSettings)
-        ++ modules;
-
+        ++ (nixvim.mkSettingsRenamedOptionModules basePluginPath settingsPath optionsRenamedToSettings);
     };
 }
diff --git a/plugins/TEMPLATE.nix b/plugins/TEMPLATE.nix
@@ -1,15 +1,11 @@
-{
-  lib,
-  pkgs,
-  ...
-}:
+{ lib, ... }:
 let
   inherit (lib.nixvim) defaultNullOpts;
 in
 lib.nixvim.neovim-plugin.mkNeovimPlugin {
   name = "my-plugin";
   originalName = "my-plugin.nvim"; # TODO replace (or remove entirely if it is the same as `name`)
-  defaultPackage = pkgs.vimPlugins.my-plugin-nvim; # TODO replace
+  package = "my-plugin-nvim"; # TODO replace
 
   maintainers = [ lib.maintainers.MyName ]; # TODO replace with your name
-Original file line number
+Diff line change
@@ Expand Up @@
     When using NixVim it is possible to encounter an error of the type `attribute 'name' missing`, for example it could look like:
+    <!-- TODO: Update example now that we use `mkPackageOption` -->
     ```
            (stack trace truncated; use '--show-trace' to show the full trace)
@@ Expand Down @@