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

Improve config files and rtp handling #1920

Open
MattSturgeon opened this issue Jul 24, 2024 · 9 comments
Open

Improve config files and rtp handling #1920

MattSturgeon opened this issue Jul 24, 2024 · 9 comments

Comments

@MattSturgeon
Copy link
Member

MattSturgeon commented Jul 24, 2024

@stasjok has an implementation here: stasjok/nixvim@optimize-rtp (commits) (last two commits), where the rtp is optimized as a "performance" option.

In the codebase I need to check wrapRc and if performance.optimizeRuntimePath is enabled. So it kind of strange to allow adding additional paths to rtp, but it works only if optimization is enabled and doesn't work otherwise...

So my thoughts are, if there is an interest in this feature, it shouldn't be performance options. Nixvim should do it by default with additional options to allow adding extra paths to rtp and packpath. It gives several benefits:

  1. nixvim will be pure by default (only paths managed by nixvim will be in rtp by default), but user always can add additional paths with options. Some plugins, like treesitter, can add the paths there automatically when nix parser management is disabled for example.
  2. filesPlugin should never be added as a plugin. This is wrong to begin with. That's because user files should always be first in rtp. When filesPlugins is in the mix with other plugins, overriding plugin files may not work (depending on sorting order).
  3. I think that there should be a default internal variable with a path to user files. By default it point to filesPlugin, but other modules (wrappers) can override it with it's own path (home-manager should set it to xdg config path that is perfectly known to it without using raw lua, like I did now, nixos can override it with etc directory with configs and so on). Final rtp is: configpath,plugins,nvimruntime,configafter.
  4. Potentially we can manage rtp partially with wrapper args, in that case we can even drop wrapRc completely and just place init.lua to filesPlugin. But this is debatable, maybe managing rtp paths as the first lines in init.lua is enough plus allow users to add paths with raw lua. Or it can mixed approach: user config path is added with command line args (so that the directory with init.lua is known to neovim), other paths are managed in init.lua.

In the end since all above as pretty large refactor and it should also change how things are managed right now, I put this task aside and decided to just change my lua code with a simple nix config in my config (in home-manager options I can get a static paths to my config files, to plugin path and to Nvim runtime, so no lua loops and matching needed, just plain vim.o.runtimepath assignment.

#1886 (comment)

Related issues:

  • We're conflating wrapRc with "isolating" the rtp - these are really separate issues.
    • wrapRc should just be about listing the init file in the wrapper args, using -u
    • Maybe we should always isolate the rtp instead of only doing it when wrapRc is enabled?
    • Is there ever a scenario where it makes sense to use nixvim with an impure config?
  • nixvim-print-init cannot show "extra files" content (i.e. filesPlugin)
  • modules/output: cleanup + refactoring #1889

Ping: @traxys @GaetanLepage

@MattSturgeon
Copy link
Member Author

Potentially we can manage rtp partially with wrapper args, in that case we can even drop wrapRc completely and just place init.lua to filesPlugin.

We may be able to do that with the -c option, which allows specifying additional vim commands from the CLI.

There's also --cmd, which is like -c except it is run before the init file.

However this wouldn't allow us to drop wrapRc, since that is mainly intended for specifying the init file to use via -u.

But this is debatable, maybe managing rtp paths as the first lines in init.lua is enough plus allow users to add paths with raw lua.

I think there's value in the rtp changes being shown transparently in nixvim-print-init.

Or it can mixed approach: user config path is added with command line args (so that the directory with init.lua is known to neovim), other paths are managed in init.lua.

Not sure what you mean by "directory with init file is known to neovim"?

My understanding is neovim computes this dir internally using APP_NAME, and using -u to specify the init file essentially means this dir is not used anyway?

But I'm not confident I fully understand this.

I think I need to read https://neovim.io/doc/user/starting.html#initialization a few times 😂
In particular, under point 7:

If Nvim was started with "-u {file}" then {file} is used as the config
and all initializations until 8. are skipped. $MYVIMRC is not set.
"nvim -u NORC" can be used to skip these initializations without
reading a file. "nvim -u NONE" also skips plugins and syntax
highlighting.

@stasjok
Copy link
Contributor

stasjok commented Jul 24, 2024

We may be able to do that with the -c option, which allows specifying additional vim commands from the CLI.

There's also --cmd, which is like -c except it is run before the init file.

However this wouldn't allow us to drop wrapRc, since that is mainly intended for specifying the init file to use via -u.

Year, I was completely wrong about it. I thought that since --cmd is run before init.lua, I can set rtp there, and Nvim will find init.lua in rtp, but that's not the case. In that case wrapRc is unavoidable (or $VIMRUNTIME can be set manually in wrapper args, but it's basically the same thing).

With this information fourth point is invalid. There is not a single reason to use --cmd args for rtp, only -u for init.lua. Settings rtp and packpath in extraConfigLuPre with mkBefore is perfectly OK. Fourth point can be changed to similar issue as filesPlugin, but with home-manager. Nixpkgs by default is adding plugin dir to rtp with set rtp^=/nix/store/xxx-vim-pack-dir. In this case plugins are first in rtp, user configs only after. My suggestion to modify rtp by default should fix that too.

@MattSturgeon
Copy link
Member Author

I'm also trying to wrap my head around whether there's ever a good reason to have a user-configured wrapRc option at all?

The only time I can think of where specifying the vimrc via -u isn't needed would be when installing via home-manager, since we install to xdg config.

Even in that scenario, I don't think setting the vimrc with -u really does any harm though?

Historical context

The wrapRc option is basically a copy of pkgs.wrapNeovimUnstable's argument of the same name. Although we implement it slightly differently to position parts of the initial file correctly.

The rtp modification to get an "isolated" config is a more recent change (c359761), that is not part of the nixpkgs implementation.

IMO the isolated rtp doesn't conceptually relate to specifying the vimrc via the wrapper, so this may also benefit from either being a separate option or not being configurable at all.

Simple option:

Can we just remove the wrapRc option entirely and always isolate the config, use -u in the wrapper, etc?

Is anyone aware of a legitimate use case for setting wrapRc = false?

@traxys
Copy link
Member

traxys commented Jul 24, 2024

None comes to mind, but maybe someone relies on it and will explain their use case if we do something with it

@stasjok
Copy link
Contributor

stasjok commented Jul 24, 2024

Is anyone aware of a legitimate use case for setting wrapRc = false?

When -u is used $MYVIMRC is not set and it is hard to get init.lua path from inside nvim. I personally don't want -u to be used in home-manager. LuaSnip for example by default loads paths relative to $MYVIMRC (maybe it fixed now, don't know), some other plugins can use it too.

@MattSturgeon
Copy link
Member Author

When -u is used $MYVIMRC is not set

True, but we could export that ourselves in the wrapper

@stasjok
Copy link
Contributor

stasjok commented Jul 24, 2024

True, but we could export that ourselves in the wrapper

Still I think it's better to put init.lua in xdg_config_home to better emulate regular, non-nix neovim. So I'd say to make this option internal/readonly and set it to true by default. Home-manager wrapper can override it to false. In other cases it's unavoidable (unless maybe settings XDG_CONFIG_HOME in the wrapper? But it's worse than setting -u, because XDG_CONFIG_HOME will be needed in internal terminal and other subprocesses). I don't think this is a big burden.

If -u would be set, I think I would want to circumvent it somehow to undo it in my config.

@MattSturgeon
Copy link
Member Author

MattSturgeon commented Jul 24, 2024

Whatever approach we use, config will be installed to environment.etc.* on NixOS and xdg.configFile.* on home-manager. If there's an equivalent for nix-darwin we should consider installing there too.

Anything in the wrapper (such as -u or exporting variables) would be in addition to that.

So I'm trying to think if there's any downsides to doing this universally, without a wrapRc option 🤔

@stasjok
Copy link
Contributor

stasjok commented Jul 24, 2024

Whatever approach we use, config will be installed to environment.etc.* on NixOS and xdg.configFile.* on home-manager. If there's an equivalent for nix-darwin we should consider installing there too.

Anything in the wrapper (such as -u or exporting variables) would be in addition to that.

So I'm trying to think if there's any downsides to doing this universally, without a wrapRc option 🤔

I'm not aware of any side effects, if neovim is ran with -u /home/user/.config/nvim/init.lua (standard XDG_CONFIG_HOME along with all other neovim configs) and $MYVIMRC is set to it's path.

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

No branches or pull requests

3 participants