nixvim: expose config for nixvim's standalone mode

[MattSturgeon]

Allow standalone nixvim users to take advantage of stylix by exposing the generated config as config.stylix.targets.nixvim.config.
I'm open to other names; maybe out, build, cfg, module?

This can be passed to the nixvim derivation's extendNixvim function, as described in the option description.

Ideally, this would be documented outside of the home-manager/nixos options page, but IMO this is fine. I guess stylix would still be installed via nixos/hm/etc even if nixvim was a standalone build...

I also considered detecting the environment the option is being used in and producing a more specific example; i.e. use home.packages in the home-manager docs' example. I figured this was overkill, and the example should be clear enough for anyone choosing to use standalone nixvim.

Here's a docs screenshot, for reference

FYI this diff is more readable with whitespace changes hidden.

[trueNAHO]

I successfully tested this PR in my standalone Home Manager configuration, which uses NixVim.

Checking config.programs ? nixvim and options.programs ? nixvim is redundant; the mkIf check had no benefit.

This now matches the pattern used elsewhere.

-- e5c11e9

The pattern originates from 7a57dff.

by exposing the generated config as config.stylix.targets.nixvim.config. I'm open to other names; maybe out, build, cfg, module?

AFAIK, the Nix convention is to interface packages as package and wrapped packages as finalPackage. Since Stylix does not wrap the NixVim config, I assume it is better to omit the final prefix, and keep config.stylix.targets.nixvim.config.

See #415 (comment) for an alternative configuration interface.

Ideally, this would be documented outside of the home-manager/nixos options page, but IMO this is fine.

Adding a Standalone section to the documentation would improve scalability of supporting more standalone programs. However, for simplicity, we should reconsider this choice. @danth, what do you think?

I guess stylix would still be installed via nixos/hm/etc even if nixvim was a standalone build...

I assume the same.

I also considered detecting the environment the option is being used in and producing a more specific example; i.e. use home.packages in the home-manager docs' example. I figured this was overkill, and the example should be clear enough for anyone choosing to use standalone nixvim.

Agreed. Also, runtime environment detection is a non-trivial task.

FYI this diff is more readable with whitespace changes hidden.

Thanks for pointing that out! I did not know GitHub supports this Git feature.

[trueNAHO]

What about inlining config.stylix.targets.nixvim.config, as it is used only once? Consequently, the public interface would change from config.stylix.targets.nixvim.config to config.programs.nixvim.

[MattSturgeon]

Apologies, I'm not sure I follow.

If you mean the read-only option should be named programs.nixvim, then that'll conflict with nixvim's option ("option declared twice" error).

If you mean programs.nixvim should be nested within the final config's value, that wouldn't work because nixvim's standalone config modules does not have the programs.nixvim prefix in its option paths.

If you mean we should do something like this, then sure - personal preference:

{
  options = /* ... */;
  config.programs.nixvim = lib.optionalAttrs (/* ... */) (lib.mkIf {/* ... */});
  config.stylix.targets.nixvim.config = {/* ... */};
}

If you mean stylix.targets.nixvim.config should change to something like stylix.programs.nixvim or stylix.configs.nixvim or stylix.out.nixvim (etc) then I'm open to that.

If you mean something else, would you mind clarifying?

[MattSturgeon]

If by inline, you mean we should remove the read-only option; adding that option is the purpose of this PR.

By extracting the attrset intended for nixvim, we allow (advanced) nixvim users not using nixvim's home-manager/NixOS modules to still apply stylix's config to a standalone nixvim (using the derivation's extendNixvim function)

[trueNAHO]

Apologies, I'm not sure I follow.

Sorry for the imprecise explanation.

If you mean the read-only option should be named programs.nixvim, then that'll conflict with nixvim's option ("option declared twice" error).

If you mean programs.nixvim should be nested within the final config's value, that wouldn't work because nixvim's standalone config modules does not have the programs.nixvim prefix in its option paths.

If you mean we should do something like this, then sure - personal preference:

{
  options = /* ... */;
  config.programs.nixvim = lib.optionalAttrs (/* ... */) (lib.mkIf {/* ... */});
  config.stylix.targets.nixvim.config = {/* ... */};
}

Thanks for the detailed analysis! Based on that, your initial proposal seems to be the right one.

If you mean stylix.targets.nixvim.config should change to something like stylix.programs.nixvim or stylix.configs.nixvim or stylix.out.nixvim (etc) then I'm open to that.

What about interfacing the config under the config.lib.stylix namespace, similar to:

stylix/stylix/palette.nix

Lines 149 to 150 in 1d3826c

	lib.stylix.colors = (base16.mkSchemeAttrs cfg.base16Scheme).override override;
	lib.stylix.scheme = base16.mkSchemeAttrs cfg.base16Scheme;

In that case, config.lib.stylix.targets.nixvim.config sounds good to me. @danth, what do you think?

[MattSturgeon]

EDIT: I misread lib.stylix as stylix.lib 😅

Making the read-only option a stylix.lib suboption would probably be neater. Although, having stylix.configs (a sibling of stylix.lib) would be even better (IMO): e.g. stylix.configs.nixvim. This pattern could then also be applied to other targets as stylix.configs.*.

This makes some sense to me, as it's kinda analogous to the final config attrset, which is passed as a module arg alongside lib, pkgs, options, etc.

I think if we go down this route, then it makes more sense to have it documented separately:
The advantage of having the option under stylix.targets.nixvim was discoverability - anyone looking at the nixvim enable option will stumble on the config option - so if they're grouped elsewhere we'd have to improve discoverability through docs (at a later date).

[MattSturgeon]

Thanks for your in-depth feedback! I'll try and respond inline below:

I successfully tested this PR in my standalone Home Manager configuration, which uses NixVim.

Just to clarify, this PR should have no effect on NixOS/home-manager nixvim installations, even ones using standalone home-manager. Rather it is intended to help users who import a pre-built "standalone" nixvim wrapped neovim package.

Checking config.programs ? nixvim and options.programs ? nixvim is redundant; the mkIf check had no benefit.
This now matches the pattern used elsewhere.
-- e5c11e9

The pattern originates from 7a57dff.

Thanks for linking the actual commit. I did understand the use of optionalAttrs is to ensure the attrset is only evaluated when the relevant option exists, since using mkIf would still evaluate the config to check it's valid even when the condition is false.

by exposing the generated config as config.stylix.targets.nixvim.config. I'm open to other names; maybe out, build, cfg, module?

AFAIK, the Nix convention is to interface packages as package and wrapped packages as finalPackage. Since Stylix does not wrap the NixVim config, I assume it is better to omit the final prefix, and keep config.stylix.targets.nixvim.config.

See #415 (comment) for an alternative configuration interface.

Thanks for your thoughts. See my questions on that thread.

I do like the idea of renaming the read-only option something like stylix.configs.nixvim or stylix.out.nixvim though.

I guess stylix would still be installed via nixos/hm/etc even if nixvim was a standalone build...

I assume the same.

I think for now it's fine to consider this a platform-specific option, since it is exposed via a nixos/hm module.

I also considered detecting the environment the option is being used in and producing a more specific example; i.e. use home.packages in the home-manager docs' example. I figured this was overkill, and the example should be clear enough for anyone choosing to use standalone nixvim.

Agreed. Also, runtime environment detection is a non-trivial task.

Well, it can be done fairly trivially; options ? home should do the trick, although you could come up with something more robust. I just figured it was over-engineering a simple example.

[curtbushko]

Friendly ping to bring these two amazing projects together.

[MattSturgeon]

If there's still interest from the maintainers, I'm happy to rebase resolving conflicts so that you can re-review.

I don't really want to spend time understanding what's changed so I can correctly resolve the conflicts, if there's no interest in reviewing/merging 😉

Let me know if you'd prefer I open another PR, so that you can review fresh without the clutter of outdated comments 🙂

[trueNAHO]

If there's still interest from the maintainers

Definitely! Btw, thanks for all your hard work on NixVim!

I'm happy to rebase resolving conflicts so that you can re-review.

I don't really want to spend time understanding what's changed so I can correctly resolve the conflicts, if there's no interest in reviewing/merging 😉

IIRC, the only blocker was how to document and interface the new functionality.

Documenting through the option's description looks good to me.

Since our last discussions, I stumbled across the config.lib.stylix.<TARGET>.<OPTION> pattern introduced in commit 01c39ac ("Create Sway module") and e43c98f ("Add an i3 module based on Sway module (#18)"):

• stylix/modules/sway.nix

  Lines 57 to 58 in 01c39ac

  	# Merge this with your bar configuration using //config.lib.stylix.sway.bar
  	lib.stylix.sway.bar = {

• stylix/modules/i3.nix

  Lines 60 to 61 in e43c98f

  	# Merge this with your bar configuration using //config.lib.stylix.i3.bar
  	lib.stylix.i3.bar = {

@danth, should we expose the NixVim config through config.lib.stylix.nixvim.config?

Let me know if you'd prefer I open another PR, so that you can review fresh without the clutter of outdated comments 🙂

It may be best to continue in this PR to keep the previous discussions for reference, but I'm open to either.

[danth]

should we expose the NixVim config through config.lib.stylix.nixvim.config?

As mentioned, this is consistent with what we are currently doing in other modules, so it's fine by me.

(Eventually, we may want to look at moving these to a more idiomatic location.)

[jasper-clarke]

Hello all,
Just wanted to check if this is still being worked on!
Thanks for the amazing work so far.

[jasper-clarke]

After a tad bit of digging I've come up with a temporary way to get this to work until this PR is merged/updated.

      # Your standalone <nixvim>.extend
      (inputs.nixvim.packages.${system}.default.extend {
        colorschemes.base16 = {
          enable = true;

          colorscheme =
            let
              colors = config.lib.stylix.colors.withHashtag;
            in
            {
              base00 = colors.base00;
              base01 = colors.base01;
              base02 = colors.base02;
              base03 = colors.base03;
              base04 = colors.base04;
              base05 = colors.base05;
              base06 = colors.base06;
              base07 = colors.base07;
              base08 = colors.base08;
              base09 = colors.base09;
              base0A = colors.base0A;
              base0B = colors.base0B;
              base0C = colors.base0C;
              base0D = colors.base0D;
              base0E = colors.base0E;
              base0F = colors.base0F;
            };
        };

        highlight =
          let
            cfg = config.stylix.targets.nixvim;
            transparent = {
              bg = "none";
              ctermbg = "none";
            };
          in
          {
            Normal = lib.mkIf cfg.transparent_bg.main transparent;
            NonText = lib.mkIf cfg.transparent_bg.main transparent;
            SignColumn = lib.mkIf cfg.transparent_bg.sign_column transparent;
          };
      })

[trueNAHO]

Hello all, Just wanted to check if this is still being worked on! Thanks for the amazing work so far.

IIRC, the only blocker was renaming the standalone NixVim interface. Now there are also merge conflicts that should to be resolved with git rebase master.

If this is all that needs to be done, feel free to take over the PR.

[jasper-clarke]

@trueNAHO
I'm more than happy to make a new PR, I am seeing an issue though.
Since this PR support for choosing either base16-nvim or mini-base16 has been introduced so more work will need to be done to continue for those options to be available.

[MattSturgeon]

Sorry I haven't looked at this in a while.

I've just rebased to resolve conflicts and to move the config definition to the proposed config.lib.stylix.nixvim.config location.

One issue I ran into is that config.lib is itself an attrsOf attrs option, and that means it cannot have sub-options merged in.

I know we'd previously assumed we would use an option-description to document this feature, but I don't think that's possible if the config is to go at that location?

If there's somewhere else I should put the documentation, or you'd prefer a different config location (that can be declared as an option), please let me know.

Also, I don't use stylix myself and I'm unfamiliar with with the repo. This means I'm not familiar with the test suite (etc).

If anyone would like to take my commits as a starting point and continue from there, that is also fine.

[jasper-clarke]

@MattSturgeon
I've got a fully functional commit working at the moment which is updated to the latest on master.
I'll create a pull request so others can double check its working for a integrated nixvim config.

[MattSturgeon]

I may be mis-remembering, but I think this might fail to merge correctly because config.lib is an attrsOf attrs type, and the element-type (attrs) doesn't support nested merging IIRC.

Again, this may be simpler to ditch the lib namespace idea and go back to declaring a dedicated read-only option somewhere. Thoughts?

[trueNAHO]

For reference, here are the most essiental parts of this discussion in chronological order:

• Ideally, this would be documented outside of the home-manager/nixos options page, but IMO this is fine.

  Adding a Standalone section to the documentation would improve scalability of supporting more standalone programs.

  -- nixvim: expose config for nixvim's standalone mode #415 (review)

• Making the read-only option a stylix.lib suboption would probably be neater. Although, having stylix.configs (a sibling of stylix.lib) would be even better (IMO): e.g. stylix.configs.nixvim. This pattern could then also be applied to other targets as stylix.configs.*.

  This makes some sense to me, as it's kinda analogous to the final config attrset, which is passed as a module arg alongside lib, pkgs, options, etc.

  I think if we go down this route, then it makes more sense to have it documented separately:
  The advantage of having the option under stylix.targets.nixvim was discoverability - anyone looking at the nixvim enable option will stumble on the config option - so if they're grouped elsewhere we'd have to improve discoverability through docs (at a later date).

  -- nixvim: expose config for nixvim's standalone mode #415 (comment)

• Since our last discussions, I stumbled across the config.lib.stylix.<TARGET>.<OPTION> pattern introduced in commit 01c39ac ("Create Sway module") and e43c98f ("Add an i3 module based on Sway module (feature: i3 module based on Sway module #18)"):

  • stylix/modules/sway.nix

    Lines 57 to 58 in 01c39ac

    	# Merge this with your bar configuration using //config.lib.stylix.sway.bar
    	lib.stylix.sway.bar = {

  • stylix/modules/i3.nix

    Lines 60 to 61 in e43c98f

    	# Merge this with your bar configuration using //config.lib.stylix.i3.bar
    	lib.stylix.i3.bar = {

  -- nixvim: expose config for nixvim's standalone mode #415 (comment)

• One issue I ran into is that config.lib is itself an attrsOf attrs option, and that means it cannot have sub-options merged in.

  I know we'd previously assumed we would use an option-description to document this feature, but I don't think that's possible if the config is to go at that location?

  If there's somewhere else I should put the documentation, or you'd prefer a different config location (that can be declared as an option), please let me know.

  -- nixvim: expose config for nixvim's standalone mode #415 (comment)

• I may be mis-remembering, but I think this might fail to merge correctly because config.lib is an attrsOf attrs type, and the element-type (attrs) doesn't support nested merging IIRC.

  Again, this may be simpler to ditch the lib namespace idea and go back to declaring a dedicated read-only option somewhere.

  -- nixvim: expose config for nixvim's standalone mode #415 (comment)

Documenting this new behaviour in a new "Standalone" section would probably be the best solution. However, considering how old this PR is, it might be better to clean this up in a follow-up PR. I assume this could be implemented by defining standalone stuff in /modules/<MODULE>/standalone.nix files. Since this probably requires touching our documentation build system, this should fall outside the scope of this PR.

Considering the mentioned config.lib limitations and that we loose the documentation that would be automatically generated from a read-only option, it might indeed be best to "ditch the lib namespace idea and go back to declaring a dedicated read-only option", as done in the very beginning of this PR: 71c7df9.

[MattSturgeon]

Documenting this new behaviour in a new "Standalone" section would probably be the best solution. However, considering how old this PR is, it might be better to clean this up in a follow-up PR.

I'm happy to find a better way to document things.

Looking at your current docs, we could probably add a "Standalone Nixvim" secction after your existing Home Manager inheritance "Configuration" section. Or alternatively on the "Tips and Tricks" page. I don't think standalone nixvim currently warrants its own page.

Considering the mentioned config.lib limitations and that we loose the documentation that would be automatically generated from a read-only option, it might indeed be best to "ditch the lib namespace idea and go back to declaring a dedicated read-only option", as done in the very beginning of this PR: 71c7df9.

Worth considering, but if we document as above then this is less important. I'm happy to tweak the implementation to one that doesn't rely on mkIf and mkMerge if it proves that the lib option doesn't support merging these wrappers.

I guess I'll have to play around with lib.evalModules and/or lib.nixosSystem in the repl to see how things work in practice, unless there's some relevant unit tests I can run?

I agree there's value in being consistent with sway and i3's outputs. Out of curiosity, how are they documented?

[MattSturgeon]

I've pushed up something that should be a reasonable starting point.

[MattSturgeon]

@trueNAHO any thoughts on the current implementation?

[trueNAHO]

Documenting this new behaviour in a new "Standalone" section would probably be the best solution. However, considering how old this PR is, it might be better to clean this up in a follow-up PR.

I'm happy to find a better way to document things.

Looking at your current docs, we could probably add a "Standalone Nixvim" secction after your existing Home Manager inheritance "Configuration" section. Or alternatively on the "Tips and Tricks" page. I don't think standalone nixvim currently warrants its own page.

Considering the mentioned config.lib limitations and that we loose the documentation that would be automatically generated from a read-only option, it might indeed be best to "ditch the lib namespace idea and go back to declaring a dedicated read-only option", as done in the very beginning of this PR: 71c7df9.

Worth considering, but if we document as above then this is less important. I'm happy to tweak the implementation to one that doesn't rely on mkIf and mkMerge if it proves that the lib option doesn't support merging these wrappers.

Agreed. The current approach of adding the "Standalone Nixvim" subsection to "Configuration" seems good.

If the standalone version works, I am fine with merging it as-is. For reference, I have tested this PR with <nixvim>.homeManagerModules.nixvim.

I guess I'll have to play around with lib.evalModules and/or lib.nixosSystem in the repl to see how things work in practice, unless there's some relevant unit tests I can run?

There are currently no tests for the NixVim module since it does not have any testbeds.

I agree there's value in being consistent with sway and i3's outputs. Out of curiosity, how are they documented?

Actually, these Sway and i3 outputs are undocumented beyond the source code comments:

• stylix/modules/sway.nix

  Lines 57 to 58 in 01c39ac

  	# Merge this with your bar configuration using //config.lib.stylix.sway.bar
  	lib.stylix.sway.bar = {

• stylix/modules/i3.nix

  Lines 60 to 61 in e43c98f

  	# Merge this with your bar configuration using //config.lib.stylix.i3.bar
  	lib.stylix.i3.bar = {

In the future, this could be improved by providing a read-only option and will probably be handled by the roadmap.