nixvim: expose config for nixvim's standalone mode
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.
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 ? nixvimandoptions.programs ? nixvimis redundant; themkIfcheck had no benefit. This now matches the pattern used elsewhere. -- e5c11e9The 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; maybeout,build,cfg,module?AFAIK, the Nix convention is to interface packages as
packageand wrapped packages asfinalPackage. Since Stylix does not wrap the NixVimconfig, I assume it is better to omit thefinalprefix, and keepconfig.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.packagesin 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.
Friendly ping to bring these two amazing projects together.
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 :wink:
Let me know if you'd prefer I open another PR, so that you can review fresh without the clutter of outdated comments :slightly_smiling_face:
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 :wink:
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 01c39ac253ae ("Create Sway module") and e43c98f9e7b6 ("Add an i3 module based on Sway module (#18)"):
- https://github.com/danth/stylix/blob/01c39ac253ae3d3170ce6befdfc89b72de601c1b/modules/sway.nix#L57-L58
- https://github.com/danth/stylix/blob/e43c98f9e7b6f999e004b35020451978fb2a5a67/modules/i3.nix#L60-L61
@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 :slightly_smiling_face:
It may be best to continue in this PR to keep the previous discussions for reference, but I'm open to either.
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.)
Hello all, Just wanted to check if this is still being worked on! Thanks for the amazing work so far.
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;
};
})
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.
@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.
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.
@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.
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
Standalonesection to the documentation would improve scalability of supporting more standalone programs.-- https://github.com/danth/stylix/pull/415#pullrequestreview-2102500050
-
Making the read-only option a
stylix.libsuboption would probably be neater. Although, havingstylix.configs(a sibling ofstylix.lib) would be even better (IMO): e.g.stylix.configs.nixvim. This pattern could then also be applied to other targets asstylix.configs.*.This makes some sense to me, as it's kinda analogous to the final
configattrset, which is passed as a module arg alongsidelib,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 understylix.targets.nixvimwas discoverability - anyone looking at the nixvimenableoption will stumble on theconfigoption - so if they're grouped elsewhere we'd have to improve discoverability through docs (at a later date).-- https://github.com/danth/stylix/pull/415#discussion_r1630383769
-
Since our last discussions, I stumbled across the
config.lib.stylix.<TARGET>.<OPTION>pattern introduced in commit 01c39ac253ae ("Create Sway module") and e43c98f9e7b6 ("Add an i3 module based on Sway module (#18)"):- https://github.com/danth/stylix/blob/01c39ac253ae3d3170ce6befdfc89b72de601c1b/modules/sway.nix#L57-L58
- https://github.com/danth/stylix/blob/e43c98f9e7b6f999e004b35020451978fb2a5a67/modules/i3.nix#L60-L61
-- https://github.com/danth/stylix/pull/415#issuecomment-2432961875
-
One issue I ran into is that
config.libis itself anattrsOf attrsoption, 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.
-- https://github.com/danth/stylix/pull/415#issuecomment-2611063864
-
I may be mis-remembering, but I think this might fail to merge correctly because
config.libis anattrsOf attrstype, and the element-type (attrs) doesn't support nested merging IIRC.Again, this may be simpler to ditch the
libnamespace idea and go back to declaring a dedicated read-only option somewhere.-- https://github.com/danth/stylix/pull/415#discussion_r1927741801
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: https://github.com/danth/stylix/commit/71c7df9b8d070a3f8b6e304e7e60718d191b12de.
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.liblimitations and that we loose the documentation that would be automatically generated from a read-only option, it might indeed be best to "ditch thelibnamespace 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?
I've pushed up something that should be a reasonable starting point.
@trueNAHO any thoughts on the current implementation?
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.liblimitations and that we loose the documentation that would be automatically generated from a read-only option, it might indeed be best to "ditch thelibnamespace 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
mkIfandmkMergeif it proves that theliboption 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.evalModulesand/orlib.nixosSystemin 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:
- https://github.com/danth/stylix/blob/01c39ac253ae3d3170ce6befdfc89b72de601c1b/modules/sway.nix#L57-L58
- https://github.com/danth/stylix/blob/e43c98f9e7b6f999e004b35020451978fb2a5a67/modules/i3.nix#L60-L61
In the future, this could be improved by providing a read-only option and will probably be handled by the roadmap.
In the future, this could be improved by providing a read-only option
I agree, read-only options under stylix.targets.«name» are the best way to handle things like this.
Sorry, I must be missing something. The reason I'm using the NixVim standalone package is because I want to be able to go nix run .#nixvim.
If I understand this PR correctly, that standalone package won't be styled. That's because the API for applying stylix to NixVim is only available inside of a NixOS or a Home Manager evaluation context.
I'm probably wrong about this. Help?
Sorry, I must be missing something. The reason I'm using the NixVim standalone package is because I want to be able to go
nix run .#nixvim.If I understand this PR correctly, that standalone package won't be styled. That's because the API for applying stylix to NixVim is only available inside of a NixOS or a Home Manager evaluation context.
I'm probably wrong about this. Help?
Correct.
Stylix itself is only really useful in scenarios where a configuration is configuring multiple programs, e.g. a home-manager configuration.
So you might have a standalone nixvim package that is installed into a styled home-manager configuration. In that scenario, you can extend the nixvim package with the stylix config.
If you're not in a home-manager configuration, then you don't have a stylix configuration.
That said, if you have a separate home-manager configuration using stylix and a standalone nixvim configuration, you could hard-code your standalone nixvim to use the home-manager configuration's config.lib.stylix.nixvim.config.
E.g. include it as one of your nixvim modules:
let
stylixModule = self.homeConfigurations.foo.config.lib.stylix.nixvim.config;
in
makeNixvim {
imports = [
./nixvim-config
stylixModule
];
}
you could hard-code your standalone nixvim to use the home-manager configuration's config.lib.stylix.nixvim.config.
Haven't thought of that. I wonder how much more eval time that would incur over standalone NixVim. I mean, if it wasn't for eval time I probably wouldn't have had standalone NixVim.
Is it not reasonable to hope that stylix would support NixVim the way it supports Home Manager?
Meanwhile I suppose I could compromise on the standalone NixVim both being not styled and being not identical to the one in Home Manager.
Is it not reasonable to hope that stylix would support NixVim the way it supports Home Manager?
It wouldn't be much effort to implement on stylix's end, but I don't think it'd be as useful as you think.
Having a stylix module for standalone nixvim would not automatically sync your styles, because they'd be defined in unrelated configurations.
I guess users could define a common stylix configuration module and import that into both your nixvim and home-manager configurations.
But it's a case of effort vs reward. If you sent a PR I would be happy to review (although I don't have rights to merge; that'd be down to stylix maintainers).