Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

neovim: add standalone module to configure it #55635

Closed
wants to merge 4 commits into from
Closed

Conversation

@teto
Copy link
Contributor

@teto teto commented Feb 12, 2019

This PR adds a module to typecheck parameters passed to build the wrapper.
It allows to generate

It adds the possibility to configure the haskell provider.
It adds a knob to disable generation of the neovim remote plugin manisfest.

Using -u to load the generated config breaks vim expectations about setting $MYVIMRC,
detecting and autoloading .nvimrc / .vimrc.
This PR changes the behavior (not backwards compatible) to fix this.
The configuration is not fully declarative anymore as it will also attempt to load the user configuration.
see here for more details #55376.

Motivation for this change

I've seen in meetups people having a hard time installing vim because the configure setting is so confusing. Using the module system makes errors more explicit, it adds some documentation. It will also make it easier to generate custom vimrc in nix-shell or in home-manager. Right now, I use tricks in my shell.nix to change the g:python3_host_prog via .nvimrc to tell neovim linters to use this project configuration. With this PR, it gets easier to just configure a new neovim wrapper for each project.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@@ -99,7 +74,7 @@ let
# Only display the log on error since it will contain a few normally
# irrelevant messages.
if ! $out/bin/nvim \
-u ${vimUtils.vimrcFile (configure // { customRC = ""; })} \
-u ${neovimrcFile} \
Copy link
Contributor Author

@teto teto Feb 12, 2019

Choose a reason for hiding this comment

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

here I should erase the customRC

@@ -113,7 +88,7 @@ let
# https://github.com/neovim/neovim/issues/9413
wrapProgram $out/bin/nvim \
--set NVIM_SYSTEM_RPLUGIN_MANIFEST $out/rplugin.vim \
--add-flags "-u ${vimUtils.vimrcFile configure}"
--add-flags "--cmd \"source ${neovimrcFile}\""
Copy link
Contributor Author

@teto teto Feb 12, 2019

Choose a reason for hiding this comment

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

alternatively I can wrap with --set VIMINIT "source vimrc" to recover the old behavior.

@kalbasit
Copy link
Member

@kalbasit kalbasit commented Feb 12, 2019

@teto can you please describe how to use this module, perhaps some documentation in the manual and/or wiki?

@teto
Copy link
Contributor Author

@teto teto commented Feb 13, 2019

@kalbasit right now I would say it just gives better error messages (typecheck) than plain nix errors when generating the vimrc.
I just updated the neovim to use it. You don't need to change anything.
The module can serve as documentation as to what parameters to pass to the neovim wrapper. The current vim configure attrset is confusing with nested sets, i.e., it's hard to configure vim without the wiki. I've seen seasoned nixos users fail to explain how to configure vim which shouldn't be the case.
Eventually I would like to get rid of configure's nested sets but right now I keep it for backwards compability and see if this can be merged first.

It seems like these kind of isolated modules don't appear in man configuration.nix, is there a way ? or that wouldn't make sense ? or generate man neovim.nix

@teto
Copy link
Contributor Author

@teto teto commented Feb 17, 2019

another motivation is to be able to merge neovim configurations. I have a global neovim with minimal python haskell/environments and when I enter a project, I have a nix-shell that generates a neovim configuration adapted for that project (with the proper python3 environment).

@teto
Copy link
Contributor Author

@teto teto commented Mar 1, 2019

@timokau any opinon ? I think this fits nix way better, like you can have a custom nvim(rc) for each nix-shell.
I haven't checked on channels:nixos-unstable but :UpdateRemotePlugins fails here for my user plugins, I can only get system ones registered. Is that the same on channels:nixos-unstable ? I can't remember.

@timokau
Copy link
Member

@timokau timokau commented Mar 1, 2019

I haven't looked at the implementation, but I don't really agree with the premise. Sorry if this sounds overly negative. That said I'm not trying to criticise you (you're doing good work) or block this from being merged, just giving my opinion.

NixOS modules have some advantages, but also significant disadvantages:

  • harder to override
  • harder to cherry-pick from a different release
  • impossible to have multiple versions installed
  • has to be re-implemented for user-level (like home-manager)

How does this enable you to have a custom nvimrc for each nix-shell? I'd argue its the other way around. You can have a custom nvim for each nix-shell right now, but I don't know how you would do that with a nixos module. It may simplify one use case (single neovim installation on nixos), but only adds to the confusion for all other use cases. In my humble opinion, nixos module should only be used for services and system configuration.

Also I think a lot of the complexity with the current way to configure neovim comes from vam and having multiple backends in general (#56338). If we only had native packages, it would significantly simplify the manual section and configuration is as easy as:

environment.systemPackages = with pkgs; [
  (neovim.override {
    configure = {
      packages.pkg = with vimPlugins; {
        start = [
          gruvbox
          base16-vim
        ];
        opt = [
          vim-nix
        ];
      };
      customRC = ''
        " something
      '';
    };
  })
]

I don't think that is particularly hard. Just unfortunately unstandardized and badly documented.

Regarding the other aspects of this PR:

Using -u to load the generated config breaks vim expectations about setting $MYVIMRC,
detecting and autoloading .nvimrc / .vimrc.
This PR changes the behavior (not backwards compatible) to fix this.

That would be nice to fix, but source doesn't set MYVIMRC either, does it? We could just set the MYVIMRC env var directly.

The configuration is not fully declarative anymore as it will also attempt to load the user configuration.

I think that is a net negative. At least as a default. It goes against the point of configuring it through nix.

It adds a knob to disable generation of the neovim remote plugin manisfest.

There shouldn't be any reason to do this. If there is, that is a bug that should be fixed. I'd be careful adding a config option to work around a bug. If it is broken right now, I'd rather have it disabled entirely until it is fixed.

@aakropotkin
Copy link
Contributor

@aakropotkin aakropotkin commented Mar 3, 2019

Can anyone confirm that setting "MYVIMRC" to anything breaks module plugins?

teto added 4 commits Mar 15, 2019
This PR adds a module to typecheck parameters passed to build the wrapper.
It allows to generate

It adds the possibility to configure the haskell provider.
It adds a knob to disable generation of the neovim remote plugin manisfest.

Using `-u` to load the generated config breaks vim expectations about setting $MYVIMRC,
detecting and autoloading .nvimrc / .vimrc.
This PR changes the behavior (not backwards compatible) to fix this.
The configuration is not fully declarative anymore as it will also attempt to load the user configuration.
see here for more details NixOS#55376.
@teto teto force-pushed the neovim_module branch from 1014fa3 to d9b9152 Mar 15, 2019
@teto
Copy link
Contributor Author

@teto teto commented Mar 15, 2019

I don't think that works as is but I merged some update from my main branch to reflect current changes.

@timokay thanks for the great feedback, I will adjust the PR once I finalize it to match the current determistic behavior. In this PR I use the nix system module rather than the nixos one, e.g., I pass a config (merged via the module system) when building neovim

For instance in my overlay I have:

   # this generates a config appropriate to work with the passed derivations
  # for instance to develop on a software from a nix-shell
  genNeovim = drvs: userConfig:
    with super;
    let
      requiredPythonModules = lib.debug.traceVal (super.python3Packages.requiredPythonModules drvs);

      # Here we generate a neovim config that allows to work with the passed 'drvs'
      # for instance adding the python propagatedBuildInputs if needed
      # or haskell ones if it's a haskell project etc.
      generatedConfig = {
        extraPython3Packages = compatFun (requiredPythonModules);
        # haskellPackages
        # TODO do the same for ruby / haskell
      };

      finalConfig = super.neovimConfig (
        super.lib.mkMerge [
          # project specific user config
          userConfig
          # my miniimal global config, when I am out of a nix-shell
          # the plugins/environments I always want available
          self.neovimDefaultConfig
          # a config generated from the input 'drvs' with an appropriate development
          # environment.
          generatedConfig
        ]
      );
    in
      wrapNeovim neovim-unwrapped-master (finalConfig);

and in my shell.nix for the python (in the future all languages) project I develop on, I have a my_neovim = genNeovim [ my_python_project] {} and it generates a neovim configuration with a python/haskell environment adapted to develop for this project.

@timokau
Copy link
Member

@timokau timokau commented Mar 17, 2019

Why do you need a nixos module for that? Couldn't you do the same thing with a base nvim and override?

@teto teto changed the title neovim: add standalone module to configure it [wip] neovim: add standalone module to configure it Mar 27, 2019
@teto teto mentioned this pull request May 22, 2019
10 tasks
@stale
Copy link

@stale stale bot commented Jun 1, 2020

Thank you for your contributions.
This has been automatically marked as stale because it has had no activity for 180 days.
If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.
Here are suggestions that might help resolve this more quickly:

  1. Search for maintainers and people that previously touched the
    related code and @ mention them in a comment.
  2. Ask on the NixOS Discourse. 3. Ask on the #nixos channel on
    irc.freenode.net.

@stale stale bot added the 2.status: stale label Jun 1, 2020
@teto
Copy link
Contributor Author

@teto teto commented Jul 4, 2020

current vim infra is still a pain for me, like some plugins do need custom configuration, we should make it possible to build vim configuration frameworks. For instance current unicode plugin should have at least:
let g:Unicode_data_directory='${final.vimPlugins.unicode-vim}/share/vim-plugins/unicode-vim/autoload/unicode' this line in config.

Also let me post https://envy.shados.net/ that may be too complex for nixpkgs but a source of possible inspiration

@stale stale bot removed the 2.status: stale label Jul 4, 2020
@timokau
Copy link
Member

@timokau timokau commented Jul 4, 2020

For what it's worth, you can get there without a nixos module: https://github.com/timokau/dotfiles/blob/c3340392081c126b5d1cf8cd3e7c519d1cdeb202/home/neovim.nix#L49

Which seems better to me since its more composable.

@teto
Copy link
Contributor Author

@teto teto commented Jul 4, 2020

Thanks for the pointer, I am not saying you can't work around nixos, but nix is in good at handling dependencies and in this case it doesn't, for instance in my unicode example, either I wish installing unicode-vim would add let g:Unicode_data_directory to my customRC without my intervention (better than patching unicode-vim).

@timokau
Copy link
Member

@timokau timokau commented Jul 6, 2020

Couldn't that still be implemented as part of the configure mechanism instead of as a nixos module?

@teto teto mentioned this pull request Sep 22, 2020
10 tasks
@ryantm ryantm marked this pull request as draft Oct 23, 2020
@stale
Copy link

@stale stale bot commented Apr 26, 2021

I marked this as stale due to inactivity. → More info

@teto teto changed the title [wip] neovim: add standalone module to configure it neovim: add standalone module to configure it Jul 28, 2021
@stale stale bot removed the 2.status: stale label Jul 28, 2021
@teto
Copy link
Contributor Author

@teto teto commented Aug 30, 2021

I still would like to leverage the nixpkgs module system to merge configurations but I am unlikely to do it soon. It's now easier to generate your own config via utilities in pkgs/applications/editors/neovim/utils.nix too. Thus closing for now.

@teto teto closed this Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants