nvim-neorg / nixpkgs-neorg-overlay

Nixpkgs overlay for Neorg and related packages
MIT License
25 stars 6 forks source link

Ideas on how to overlay vimPlugins and tree-sitter-grammars #3

Closed pschyska closed 2 years ago

pschyska commented 2 years ago

I have some remarks on the overlay technique, and thought about on how to clean it up a bit.

vimPlugins

{ vimPlugins = prev.vimPlugins // { … } }

should probably replaced with:

{
  vimPlugins = prev.vimPlugins.extend (f: p: {
    neorg = p.neorg.overrideAttrs (a: {
      dependencies = [ f.plenary-nvim ];
      version = neorg.rev;
      src = neorg;
    });
    neorg-telescope = final.vimUtils.buildVimPluginFrom2Nix {
      dependencies = [ f.telescope-nvim f.neorg ];
      pname = "neorg-telescope";
      version = neorg-telescope.rev;
      src = neorg-telescope;
    };
  });
}

I got bitten with overriding vimPlugins without extend before, because when I was using non-overriden plugins that had some overriden plugins as dependencies, they ended up in the pack path twice, leading to conflicts. extend ensures that there is only ever one derivation. Right now this is a theoretical issue though, as no plugin in nixpkgs depends on neorg, and neorg-telescope is not in nixpkgs yet, but this should future-proof for when these conditions change. I'm not 100% sure if neorg above should be defined with overrideAttrs though, I think a fresh buildVimPluginFrom2Nix would also work.

tree-sitter-grammars

I failed so far to properly overlay them, just overriding the attrs that are visible in the end (e.g. tree-sitter.{built,all}Grammars, tree-sitter-grammars etc.) didn't have the intended effect (in particular, the withPlugins functions get passed-in non-overriden versions of my overriden grammars). I'm not completely sure why this is, but I just couldn't make it work. Therefore I'd suggest to not do it, and put the grammars somewhere top-level. It's not an issue to refer to "external" grammars in the withPlugins closure, e.g. from my config:

      (nvim-treesitter.withPlugins (p:
        (attrValues (removeAttrs p [
          "tree-sitter-perl" # highlighting issues, e.g. in "${…}", also missing indent
          "tree-sitter-norg"
          "tree-sitter-bash"
          "tree-sitter-kotlin"
        ])) ++ [
          final.tree-sitter-norg
          final.tree-sitter-norg-meta
          final.p-tree-sitter-grammars.tree-sitter-bash
          final.p-tree-sitter-grammars.tree-sitter-kotlin
        ]))

I experimented with overriding nvim-treesitter in a way that would allow the overlay to express that neorg depends on it with the neorg parsers added automatically. It broke when the user also put nvim-tressiter with some plugins top-level though, but I didn't give up yet. It would look something like this:

      with inputs;
      let
        grammars = {
          tree-sitter-norg = norg.defaultPackage.${final.system};
          tree-sitter-norg-meta = norg-meta.defaultPackage.${final.system};
        };
      in {
        vimPlugins = prev.vimPlugins.extend (f: p: {
          nvim-treesitter = p.nvim-treesitter.overrideAttrs (a: {
            passthru.withPlugins = grammarFn:
              a.passthru.withPlugins (plugins:
                (grammarFn plugins)
                ++ [ grammars.tree-sitter-norg grammars.tree-sitter-norg-meta ]);
          });
          neorg = p.neorg.overrideAttrs (a: {
            dependencies = (a.dependencies or [ ])
              ++ [ f.plenary-nvim (f.nvim-treesitter.withPlugins (_: [ ])) ];
            version = neorg.rev;
            src = neorg;
          });
          neorg-telescope = final.vimUtils.buildVimPluginFrom2Nix {
            dependencies = [ f.telescope-nvim f.neorg ];
            pname = "neorg-telescope";
            version = neorg-telescope.rev;
            src = neorg-telescope;
          };
        });
      }

(N.B. nvim-treesitter.withPlugins always adds norg grammars to the list, but I don't see a reason for users pulling in this overlay to not want that)

This would then enable to simplify the instructions to just adding vimPlugins.neorg and the tree-sitter business would all be solved automatically.

What do you think? If that sounds good to you I'd like to take a stab and prepare another PR.

Cheers

bandithedoge commented 2 years ago

I'm out of my machine for the rest of today, so you could try this.