plugins/alpha: migrate to mkNeovimPlugin #3040

HeitorAugustoLN · 2025-02-27T22:24:24Z

Tracking: #2638

MattSturgeon · 2025-02-27T22:40:34Z

plugins/by-name/alpha/default.nix

+      plugins.alpha.luaConfig.content =
+        optionalString themeDefined "require('alpha').setup(${toLuaObject cfg.theme})\n"
+        + "require('alpha.term')";


I'm a little confused. You have a settings option, but you are also manually calling setup and passing it a theme option.

The idea of settings is that it is the nix representation of the lua passed to setup.

Usually, mkNeovimPlugin will take responsibility for calling setup and passing it the settings.

.setup needs to be manually called when using theme option. Because that is the only way we can set pre-defined themes. That's why, theme and settings.layout options can't be used at the same time

If so, that also implies:

settings as a whole must not be used when a theme is defined

setup must always be called manually, either passing it settings or theme

Interesting, the upstream examples show a different usage:

require('alpha').setup(require('alpha.themes.dashboard').config)

They are passing pre-defined settings to setup; getting those settings from a theme module.

It is the same usage, the theme option is being converted here:

nixvim/plugins/by-name/alpha/default.nix

Lines 196 to 201 in c83415c

apply =

value:

if builtins.isString value then

lib.nixvim.mkRaw ''require("alpha.themes.${value}").config''

else

value;

This is now solved as settings can accept a rawLua type.
Hence, the user can do:

plugins.alpha = { enable = true; settings.__raw = "require('alpha.themes.dashboard').config"; };

plugins/by-name/alpha/default.nix

MattSturgeon

Sorry, I always struggle to review mkNeovimPlugin migrations as the diff ends up very noisy.

I also had most of this review "pending" for a while as I forgot to hit "submit" 😅

MattSturgeon · 2025-03-02T04:00:34Z

plugins/by-name/alpha/default.nix

+
+  maintainers = [ lib.maintainers.HeitorAugustoLN ];
+
+  callSetup = false;


Typically we have an explanatory comment.

I moved callSetup = false; right above extraConfig to make its purpose obvious.

MattSturgeon · 2025-03-02T04:05:58Z

plugins/by-name/alpha/default.nix

+  };

-      extraPlugins = [ cfg.package ];
+  optionsRenamedToSettings = [


This should have a comment saying when it was added.

Also, we need to update the test cases. See CI failures: https://buildbot.nix-community.org/#/builders/2885/builds/3656/steps/1/logs/stdio

MattSturgeon · 2025-06-02T15:57:12Z

plugins/by-name/alpha/default.nix

-          require('alpha.term')
-        '';
+      plugins.alpha.luaConfig.content = ''
+        require('alpha').setup(${toLuaObject (if themeDefined then cfg.theme else cfg.settings)});


This feels like it could be confusing to users. I wonder if we should have it so that we do this in the module system?

settings = mkDerivedConfig opts.theme ( value: if builtins.isString value then mkRaw ''require("alpha.themes.${value}").config'' else value );

However, that'd require refactoring settings to a maybeRaw type.

As a minimum, the weird behaviour of discarding all configured settings if a theme is defined should be documented somewhere.

I still feel like this behaviour of settings only being used when there is no theme is a bad design 🤔

plugins/by-name/alpha/default.nix

MattSturgeon · 2025-09-14T14:54:11Z

plugins/by-name/alpha/default.nix

-          require('alpha.term')
-        '';
+      plugins.alpha.luaConfig.content = ''
+        require('alpha').setup(${toLuaObject (if themeDefined then cfg.theme else cfg.settings)});


I still feel like this behaviour of settings only being used when there is no theme is a bad design 🤔

MattSturgeon · 2025-09-16T21:06:53Z

plugins/by-name/alpha/default.nix


-      assertions = lib.nixvim.mkAssertions "plugins.alpha" [
+  extraConfig =
+    opts: cfg:


Suggested change

opts: cfg:

cfg: opts:

MattSturgeon

(Note: there are a few older unresolved threads still)

MattSturgeon · 2025-09-16T21:09:51Z

plugins/by-name/alpha/default.nix

+            !(
+              themeDefined
+              && (opts.settings.highestPrio == 1500 && builtins.length opts.settings.definitions == 1)
+            );


I'm probably mis-understanding this logic. But should the assertion be themeDefined implies settings not defined?

Suggested change

!(

themeDefined

&& (opts.settings.highestPrio == 1500 && builtins.length opts.settings.definitions == 1)

);

themeDefined ->

(opts.settings.highestPrio == 1500 && builtins.length opts.settings.definitions == 1);

GaetanLepage · 2025-10-26T13:32:20Z

This should now be ready!

MattSturgeon reviewed Feb 27, 2025

View reviewed changes

HeitorAugustoLN force-pushed the alpha-mkneovimplug branch 2 times, most recently from c83415c to 3ea66f1 Compare February 27, 2025 23:11

MattSturgeon reviewed Feb 27, 2025

View reviewed changes

plugins/by-name/alpha/default.nix Outdated Show resolved Hide resolved

MattSturgeon reviewed Feb 27, 2025

View reviewed changes

plugins/by-name/alpha/default.nix Outdated Show resolved Hide resolved

HeitorAugustoLN force-pushed the alpha-mkneovimplug branch 2 times, most recently from cbddcc4 to 0337a66 Compare February 28, 2025 19:14

MattSturgeon reviewed Mar 1, 2025

View reviewed changes

plugins/by-name/alpha/default.nix Show resolved Hide resolved

HeitorAugustoLN force-pushed the alpha-mkneovimplug branch from 0337a66 to 03a6663 Compare March 1, 2025 19:42

MattSturgeon reviewed Jun 2, 2025

View reviewed changes

This comment was marked as resolved.

Sign in to view

saygo-png mentioned this pull request Jul 16, 2025

[TRACKING] Use mkNeovimPlugin treewide #2638

Open

4 tasks

This comment was marked as resolved.

Sign in to view

HeitorAugustoLN force-pushed the alpha-mkneovimplug branch from 03a6663 to d12fba9 Compare September 13, 2025 21:11

This comment was marked as resolved.

Sign in to view

HeitorAugustoLN force-pushed the alpha-mkneovimplug branch from d12fba9 to 6fa4aaa Compare September 14, 2025 13:49

MattSturgeon reviewed Sep 14, 2025

View reviewed changes

HeitorAugustoLN force-pushed the alpha-mkneovimplug branch from 6fa4aaa to faa3eea Compare September 14, 2025 15:14

MattSturgeon reviewed Sep 16, 2025

View reviewed changes

GaetanLepage force-pushed the alpha-mkneovimplug branch 3 times, most recently from 7945e0f to 370ea63 Compare October 26, 2025 13:17

GaetanLepage requested review from MattSturgeon and khaneliman October 26, 2025 13:20

GaetanLepage force-pushed the alpha-mkneovimplug branch from 370ea63 to d64bb14 Compare October 27, 2025 09:38

plugins/alpha: migrate to mkNeovimPlugin

caaa371

GaetanLepage force-pushed the alpha-mkneovimplug branch from d64bb14 to caaa371 Compare October 27, 2025 09:39

	apply =
	value:
	if builtins.isString value then
	lib.nixvim.mkRaw ''require("alpha.themes.${value}").config''
	else
	value;


		maintainers = [ lib.maintainers.HeitorAugustoLN ];

		callSetup = false;

Uh oh!

Uh oh!

plugins/alpha: migrate to mkNeovimPlugin #3040

Are you sure you want to change the base?

plugins/alpha: migrate to mkNeovimPlugin #3040

Uh oh!

Conversation

HeitorAugustoLN commented Feb 27, 2025 • edited by GaetanLepage Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MattSturgeon Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MattSturgeon Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MattSturgeon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MattSturgeon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GaetanLepage commented Oct 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

HeitorAugustoLN commented Feb 27, 2025 •

edited by GaetanLepage

Loading

MattSturgeon Feb 27, 2025 •

edited

Loading

MattSturgeon Feb 27, 2025 •

edited

Loading