home-manager icon indicating copy to clipboard operation
home-manager copied to clipboard

programs.neovim: Add initExtra option (2)

Open viniciusmuller opened this issue 4 years ago • 22 comments

Description

The last PR was truly messy (#2244) so I've closed it and I'm opening a new one now.

Checklist

  • [X] Change is backwards compatible.

  • [X] Code formatted with ./format.

  • [ ] Code tested through nix-shell --pure tests -A run.all.

  • [X] Test cases updated/added. See example.

  • [X] 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.

ping @berbiche

viniciusmuller avatar Oct 09 '21 16:10 viniciusmuller

With neovim we have people writing their config in lua/fennel/teal; I would like to supporting these approaches in neovim. This means we may have to modify the option aftwards. @berbiche I'll let you handle the reviews.

teto avatar Oct 09 '21 19:10 teto

With neovim we have people writing their config in lua/fennel/teal; I would like to supporting these approaches in neovim. This means we may have to modify the option aftwards. @berbiche I'll let you handle the reviews.

I always assumed that the easiest solution to this in the future would be to add an option specifying the output filetype. We can reuse the existing initExtra option but put it into init.lua or some other file format instead.

This possibly gets a little more complicated if you could have both an init.vim and init.lua for example, but regardless still seems doable without modifying the behavior of this option in particular.

kclejeune avatar Oct 10 '21 17:10 kclejeune

I always assumed that the easiest solution to this in the future would be to add an option specifying the output filetype. We can reuse the existing initExtra option but put it into init.lua or some other file format instead.

May we discuss this design decision here? It would be nice as I'm as interested in the initExtra as in using fennel in my configuration. Personally I'm really fond of an option specifying the filetype!

ratsclub avatar Oct 14 '21 13:10 ratsclub

I would be happy for someone to take on this but we should agree on the design first. My current take is:

  • to add a freeform "language" option to the plugin submodule (that would default to vim)
  • add a function that collects config with same language and does what it wants with it. By default it would write to init.LANGUAGE

As for the initExtra, someone mentioned putting its initExtra as a fake plugin. Looks like an interesting workaround.

I am scared to add something to complex for users and/or to maintain. At the same time as someone with a complex hybrid lua/viml config (and looking forward to add some fennel), I would still like to make it possible for advanced users.

teto avatar Oct 17 '21 23:10 teto

I was bitten by this issue after release-21.11. I remap my leader from \ to Space, and since it seems programs.neovim.extraConfig is called later than the plugins my config stopped working (it used to work before).

Could we have this PR merged and backport to release-21.11? It maybe not perfect but at least it solves the issue.

thiagokokada avatar Nov 29 '21 22:11 thiagokokada

I was bitten by this issue after release-21.11. I remap my leader from \ to Space, and since it seems programs.neovim.extraConfig is called later than the plugins my config stopped working (it used to work before).

For now I work around the same Issue by prepanding the first plugins configuration with my nvimrc. E.g.

programs.neovim = {
  ...
  plugins = [
    {
      plugin = pkgs.vimPlugins.gruvbox;
      config = builtins.readFile ./config/nvimrc + builtins.readFile ./config/gruvbox.vim;
    }
  ];
};

GlancingMind avatar Dec 07 '21 16:12 GlancingMind

@teto following the merge of https://github.com/nix-community/home-manager/pull/2637, what should be changed here to have this merged?

berbiche avatar Feb 20 '22 05:02 berbiche

I am annoyedby the set nocompatible that creeps in the generated init.vim . Also now that plugins can specify the language of their config, it would be nice to be able to precise this in option, @selene57 might have a go at this https://github.com/nix-community/home-manager/pull/2716

teto avatar Feb 27 '22 12:02 teto

@teto I am annoyedby the set nocompatible that creeps in the generated init.vim . Also now that plugins can specify the language of their config, it would be nice to be able to precise this in option, @selene57 might have a go at this #2716

If we drop the the set nocompatible line and keep the Home Manager comment, do you think we could merge this?

berbiche avatar Apr 17 '22 22:04 berbiche

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.
If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

stale[bot] avatar Jul 17 '22 01:07 stale[bot]

Is this still on the table?

expipiplus1 avatar Sep 04 '22 07:09 expipiplus1

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.
If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

stale[bot] avatar Dec 03 '22 10:12 stale[bot]

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.
If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

stale[bot] avatar Mar 08 '23 16:03 stale[bot]

I was bitten by this issue after release-21.11. I remap my leader from \ to Space, and since it seems programs.neovim.extraConfig is called later than the plugins my config stopped working (it used to work before).

For now I work around the same Issue by prepanding the first plugins configuration with my nvimrc. E.g.

programs.neovim = {
  ...
  plugins = [
    {
      plugin = pkgs.vimPlugins.gruvbox;
      config = builtins.readFile ./config/nvimrc + builtins.readFile ./config/gruvbox.vim;
    }
  ];
};

Thank you for sharing your answer, it saved me a lot of time. In case it helps anyone else, if you have your entire configuration defined in an init.lua, you can separate it into 2 groups, the "core" configurations (that you need to run at the beginning) and the plugins config. That done, you will end up with something like:

programs.neovim = {
      ...
      extraConfig = ":luafile ~/.config/nvim/append_config.lua";
      plugins = with pkgs.vimPlugins; [
        { plugin = gruvbox-nvim;
          config = ("lua << EOF\n" + builtins.readFile ./nvim/preppend_config.lua + "EOF");
        }
       ...
       nvim-cmp
       etc
        ...
     ];
};

Where preppend_config will load the core, and append_config will take care of the plugins at the end.

However, this feels quite convoluted. would it be possible for you to please reconsider the initExtra option? I really consider it an option that carries enough value to create it!

Anyway, thank you and best regards!

Dioprz avatar Apr 30 '23 10:04 Dioprz

FWIW, I use this almost trivial patch. If someone wants to neaten it up (change the name to something more descriptive than initExtra, allow it to accept lua, add tests) then please feel free to take it.

diff --git a/modules/programs/neovim.nix b/modules/programs/neovim.nix
index df4b487d..28abf1e0 100644
--- a/modules/programs/neovim.nix
+++ b/modules/programs/neovim.nix
@@ -98,6 +98,14 @@ in {
     programs.neovim = {
       enable = mkEnableOption "Neovim";

+      initExtra = mkOption {
+        type = types.path;
+        default = null;
+        description = ''
+          Configuration to insert at the top of <filename>$XDG_CONFIG_HOME/nvim/init.vim</filename>
+        '';
+      };
+
       viAlias = mkOption {
         type = types.bool;
         default = false;
@@ -401,6 +409,8 @@ in {
         (map (x: x.runtime) pluginsNormalized) ++ [{
           "nvim/init.lua" = let
             luaRcContent =
+              lib.optionalString (cfg.initExtra != null)
+              "vim.cmd [[source ${cfg.initExtra}]]\n" +
               lib.optionalString (neovimConfig.neovimRcContent != "")
               "vim.cmd [[source ${
                 pkgs.writeText "nvim-init-home-manager.vim"

expipiplus1 avatar May 01 '23 03:05 expipiplus1

Why not extraConfig = mkBefore ...?

ncfavier avatar May 01 '23 09:05 ncfavier

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.
If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

stale[bot] avatar Jul 30 '23 16:07 stale[bot]

@ncfavier because extraConfig itself is always placed after the plugin configs, so even if something is at the top of extraConfig it's too late.

expipiplus1 avatar Aug 19 '23 03:08 expipiplus1

ideally nixpkgs' code should be rewritten with the nixos module so that we dont have to create extra options and can just do mkBefore/mkAfter.

I am counting on https://github.com/NixOS/nixpkgs/pull/237425 to make overrides easier too.

teto avatar Aug 19 '23 14:08 teto

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.
If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

stale[bot] avatar Nov 18 '23 07:11 stale[bot]

dont update this PR yet, there is another approach I would like to take now that most of it is merged in nixpkgs.

teto avatar Nov 21 '23 23:11 teto

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.
If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

stale[bot] avatar Feb 20 '24 03:02 stale[bot]