Skip to content

neovim: autogenerate config.lua file sourced to init.vim#2716

Merged
teto merged 1 commit intonix-community:masterfrom
selene57:auto-generate-neovim-config-lua
Feb 23, 2022
Merged

neovim: autogenerate config.lua file sourced to init.vim#2716
teto merged 1 commit intonix-community:masterfrom
selene57:auto-generate-neovim-config-lua

Conversation

@selene57
Copy link
Copy Markdown
Contributor

@selene57 selene57 commented Feb 11, 2022

Description

The neovim module now automatically adds a config.lua file and sources it at the bottom of the init.vim configuration file, when the aggregate plugin configurations for the lua type have non-zero length. This adds the remaining functionality (for lua plugins) that #2637 set out to add to the neovim module. Previous to this change, the lua files were not sourced to the init.vim file, even if you created the lua files yourself in your home-manager configuration file.

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all. (via make test TEST=all)

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

    See CONTRIBUTING for more information and recent commit messages for examples.

  • If this PR adds a new module

    • Added myself as module maintainer. See example.

    • Added myself and the module files to .github/CODEOWNERS.

@selene57 selene57 requested a review from rycee as a code owner February 11, 2022 10:55
@selene57
Copy link
Copy Markdown
Contributor Author

I tried running the test suite, but attribute shellDryRun was missing. Given that I didn't go anywhere near those files, I'm going to assume something is off or faulty there?

@selene57
Copy link
Copy Markdown
Contributor Author

Looks like there's an upstream remarshal package error in nixpkgs. It's supposedly fixed on master. NixOS/nixpkgs#159074

@sumnerevans sumnerevans requested review from berbiche and teto February 15, 2022 03:25
@sumnerevans
Copy link
Copy Markdown
Contributor

Yep, need it to hit nixpkgs-unstable: https://nixpk.gs/pr-tracker.html?pr=159074

@teto
Copy link
Copy Markdown
Collaborator

teto commented Feb 15, 2022

sounds sensible. I have something similiar in my config except that I've named the file init.generated.lua.
Tests would be a nice addition.
In case neovimConfig.neovimRcContent is empty and config.programs.neovim.generatedConfigs.lua is not, we could even write config.programs.neovim.generatedConfigs.lua to `init.lua. This adds some complexity so not sure that's worth it, we can look into it later.

@selene57
Copy link
Copy Markdown
Contributor Author

selene57 commented Feb 15, 2022

sounds sensible. I have something similiar in my config except that I've named the file init.generated.lua. Tests would be a nice addition. In case neovimConfig.neovimRcContent is empty and config.programs.neovim.generatedConfigs.lua is not, we could even write config.programs.neovim.generatedConfigs.lua to `init.lua. This adds some complexity so not sure that's worth it, we can look into it later.

So the use case you refer to, where we would automatically generate a init.lua file has some issues. The first being that neovim.generatedConfigs.lua will automatically be empty if neovimConfig.neovimRcContent is empty. The reason for this being the case is that neovim.generatedConfigs.lua only aggregates lua code found in plugins, while having plugins requires the package and runtime support, which mandates 2 minimum lines in neovimConfig.neovimRcContent. The second being that neovim.generateConfigs.lua does not have the package and runtime support lines automatically added at the top.

Ideally, I agree with you. It would be great to have the neovim module support lua-only configs as that is quickly becoming the standard (if it not already is). Ideally, we'd also figure out a way to add lua module support for configuration, as it is pretty common place for lua based configs to have quite a lot of customization done outside of plugins (and sending it to a single file is a bit messy). Correct me if I'm wrong, but I think this would require a rewrite of this part of the code:

neovimConfig = pkgs.neovimUtils.makeNeovimConfig {
      inherit (cfg)
        extraPython3Packages withPython3 withNodeJs withRuby viAlias vimAlias;
      configure = cfg.configure // moduleConfigure;
      plugins = (map suppressNotVimlConfig pluginsNormalized)
        ++ optionals cfg.coc.enable [{ plugin = pkgs.vimPlugins.coc-nvim; }];
      customRC = cfg.extraConfig;

https://github.com/NixOS/nixpkgs/blob/master/pkgs/applications/editors/neovim/utils.nix is where makeNeovimConfig comes from. I would be happy to do a more robust deep dive into this and try to come up with a solution that works for a wider breadth of neovim configs. Let me know, if we think that would be useful enough?

Also, it seems that another pull request was just submitted attempting to work on this too: I'm linking that here (#2723).

@teto
Copy link
Copy Markdown
Collaborator

teto commented Feb 16, 2022

The second being that neovim.generateConfigs.lua does not have the package and runtime support lines automatically added at the top.

This could be added to either lua or vimscript, not a problem IMO.

You can look at nixpkgs nixos/modules/programs/neovim.nix:
They have some code to generate a runtime.

  runtime' = filter (f: f.enable) (attrValues cfg.runtime);
  runtime = pkgs.linkFarm "neovim-runtime" (map (x: { name = x.target; path = x.source; }) runtime');

I would like it if we could have this as a parameter of makeNeovimConfig as you suggest.

@teto
Copy link
Copy Markdown
Collaborator

teto commented Feb 16, 2022

I would like before merging a more specific name than lua require('config')'' that someone may already use. I think init.generated.lua is less likely to conflict with existing config. Or init.home-manager.lua if you prefer

@selene57 selene57 force-pushed the auto-generate-neovim-config-lua branch 2 times, most recently from 8229801 to 833bd92 Compare February 17, 2022 05:01
@selene57
Copy link
Copy Markdown
Contributor Author

I would like before merging a more specific name than lua require('config')'' that someone may already use. I think init.generated.lua is less likely to conflict with existing config. Or init.home-manager.lua if you prefer

I have amended my commit/PR to change the config.lua file name to init-home-manager.lua. There were some pathing errors when I tried your original suggestions of init.home-manager.lua and init.generated.lua.

I hope this change is sufficient to suggest this point? If not, please let me know so I can make further changes so this can be merged. Appreciate it!

@selene57
Copy link
Copy Markdown
Contributor Author

The second being that neovim.generateConfigs.lua does not have the package and runtime support lines automatically added at the top.

This could be added to either lua or vimscript, not a problem IMO.

You can look at nixpkgs nixos/modules/programs/neovim.nix: They have some code to generate a runtime.

  runtime' = filter (f: f.enable) (attrValues cfg.runtime);
  runtime = pkgs.linkFarm "neovim-runtime" (map (x: { name = x.target; path = x.source; }) runtime');

I would like it if we could have this as a parameter of makeNeovimConfig as you suggest.

After this gets merged, I think I'll tackle this next. I'd also like to add the option to add arbitrary lua code in the form of modules and source the files appropriately.

Thanks!

@teto
Copy link
Copy Markdown
Collaborator

teto commented Feb 17, 2022

arf the CI doesn't pass because of an issue in nixpkgs (remarshal build is fixed in master so we have to wait for it to reach unstable). I want to merge but I dont think I can with the failing CI.
As for the runtime improvements, I am wary that it adds too much complexity instead of letting the users create files. So might be best to present what it intends to solve in a ticket, with some explanation of how it would be used, what are the necessary changes.

@selene57
Copy link
Copy Markdown
Contributor Author

arf the CI doesn't pass because of an issue in nixpkgs (remarshal build is fixed in master so we have to wait for it to reach unstable). I want to merge but I dont think I can with the failing CI. As for the runtime improvements, I am wary that it adds too much complexity instead of letting the users create files. So might be best to present what it intends to solve in a ticket, with some explanation of how it would be used, what are the necessary changes.

It looks like the tests are passing again; would you mind rerunning the CI?

@teto
Copy link
Copy Markdown
Collaborator

teto commented Feb 22, 2022

I can't unless you rebase or amend your last commit. Do it and I will merge it (no need to ping me if tests pass)

@selene57 selene57 force-pushed the auto-generate-neovim-config-lua branch from 833bd92 to 4c63e99 Compare February 23, 2022 09:24
@selene57
Copy link
Copy Markdown
Contributor Author

I can't unless you rebase or amend your last commit. Do it and I will merge it (no need to ping me if tests pass)

Sorry for the ping, but the tests won't run until they are approved by someone. I rebased, so hopefully this will do the trick to pass CI now.

Thanks!

@teto
Copy link
Copy Markdown
Collaborator

teto commented Feb 23, 2022

seems to be a valid error here https://github.com/nix-community/home-manager/runs/5302698665?check_suite_focus=true#step:9:868 . Btw you can run the tests locally with make test

@selene57 selene57 force-pushed the auto-generate-neovim-config-lua branch from 4c63e99 to 6ccacd2 Compare February 23, 2022 12:21
@selene57
Copy link
Copy Markdown
Contributor Author

seems to be a valid error here https://github.com/nix-community/home-manager/runs/5302698665?check_suite_focus=true#step:9:868 . Btw you can run the tests locally with make test

Thank you make test TEST=all worked. I've amended my commit to fix the bug. The test doesn't have any lua plugins so the lua attribute wasn't being made (because the attributes are being automatically made via groupBy). I've replaced the checks for length on the lua attribute with a hasAttr call, making those checks now safe and able to pass the tests even when there are no lua plugins.

All tests are passing locally now. Thanks!

@teto teto merged commit 0b1745b into nix-community:master Feb 23, 2022
@teto
Copy link
Copy Markdown
Collaborator

teto commented Feb 23, 2022

so next step would be if no viml config, generate an "init.lua" instead of an "init-home-manager.lua" ?

@selene57 selene57 deleted the auto-generate-neovim-config-lua branch February 23, 2022 22:15
@selene57
Copy link
Copy Markdown
Contributor Author

so next step would be if no viml config, generate an "init.lua" instead of an "init-home-manager.lua" ?

I think so, but I also think it might be worth making the extraConfig option typeable between a viml and lua (with viml being the default so as to not break people's configs). Otherwise generating an init.lua file would give you no way to source additional lua code for your config (outside of the plugin specific ones, which wouldn't be very idiomatic) for things like setting , line numbers, etc. Does that sound like a reasonable approach?

I'm going to try to get that up and working this weekend, if nobody else starts working on it first.

Thanks for the merge btw!

@teto
Copy link
Copy Markdown
Collaborator

teto commented Feb 24, 2022

Yeah extraConfig could be of type pluginWithConfigType (ignoring the "optional" part)

@berbiche
Copy link
Copy Markdown
Member

Yeah extraConfig could be of type pluginWithConfigType (ignoring the "optional" part)

Though it would be odd to have an extraConfig.config and extraConfig.type option.
I'd prefer if extraConfig remained a string to be consistent with all the other modules.
The disadvantage of keeping this consistency is that we would need to add a new option e.g. extraConfigType.

What do you think?

neovimConfig.neovimRcContent;
};
xdg.configFile."nvim/lua/init-home-manager.lua" =
mkIf (hasAttr "lua" config.programs.neovim.generatedConfigs) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

When exactly does this file get generated? generatedConfigs is readonly, and I cannot think of a scenario when it would have a lua attr?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

if you set your plugin like this:

    {
      plugin = urlview-nvim;
      type = "lua";
	  config = ''
		'';
    }

@mlyxshi
Copy link
Copy Markdown
Contributor

mlyxshi commented May 17, 2022

@teto

so next step would be if no viml config, generate an "init.lua" instead of an "init-home-manager.lua" ?

I prefer the pure lua way.

Because the modular design of lua(require),
confugure ~/.config/nvim/lua by xdg.configFile."nvim/lua".source = ./lua;
is more friendly to manage modular and modern nvim setup.

https://github.com/mlyxshi/flake/blob/main/home/nvim/nvim.nix

I'm not familiar with nix lang right now, could you please implement the init.lua?

@teto teto mentioned this pull request Aug 22, 2022
7 tasks
teto pushed a commit to teto/home-manager that referenced this pull request Aug 22, 2022
spacekookie pushed a commit to spacekookie/home-manager that referenced this pull request Feb 10, 2023
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.

6 participants