plugins/by-name: init

[MattSturgeon]

Add support for automatically importing any directories under plugins/by-name.

Documentation

• update CONTRIBUTING.md
• add a plugins/by-name/README.md (maybe?)

Validation

A test is included that does some validation on the by-name directory and plugin-options* enabled within it:

• run the validation tests in the git pre-commit hook
• by-name should contain only directories
• plugin name should match directory name where its options are declared
• option path starts with either "colorschemes" or "plugins"
• Ensure a matching testfile exists in the correct location
• by-name plugins should have a default.nix
  • This should be detected anyway as the import will fail if there's no default.nix file
• one plugin per directory
  • This seems an unnecessary check. It's probably not worth dealing with the complexity...

We could also have a CI workflow running on new PRs only, that validates whether new plugins are added to by-name instead of a category.

* "Plugin options" are collected by looking for options matching *.*.enable

Migration

See #2164 (comment)

I'd like an overall consensus on this before working on such migrations. See "Design decisions" below.

Future tooling

I think it'd be useful to have a devshell command that interactively builds a new plugin module based on the template.

You would be prompted for things like "type" (vim, lua, etc) "name", "pkg name" "setup fn", "globals prefix", and it'd automatically produce a minimal module file.

$ create-plugin
What's the plugin's name? 
> hello

What's the plugin type? (1. neovim (i.e. `require().setup()`) 2. vim with globals 3. vim without settings)
> 1

Current configuration:
1. name = "hello"
2. type = lua/neovim with setup function
3. luaName = "hello"
4. pkg = pkgs.vimPlugins.hello
5. setup = ".setup"

Would you like to change anything? (type a line number to modify it, or <return> to finish)
>

Design decisions

• We don't need a prefix directory (like in nixpkgs) because we are dealing with a smaller scale
  • By prefix, I mean the "na" directory in pkgs/by-name/na/name/package.nix.
  • We can just have plugins/by-name/<name>/default.nix to keep things simple.
  • Nixpkgs has around 500 prefixes alone in pkgs/by-name, while we have around 200 files total matching plugins/*/*.nix.
  • Having a prefix level would complicate both the implementation and usage, e.g. actually searching for a plugin's module.
  • Tab completion for plugins/by-name/examp<TAB> should be easier without a prefix.
• We shouldn't need to add by-name plugins to an imports list
  • It is trivial to read the directory and fold that into a list of paths to import.
  • This is one less thing for new contributors to worry about when adding a plugin module.
• We should exclusively use directories with default.nix files.
  • This adds consistency.
  • It is one less judgement call to make on new plugins (i.e. whether to use foo/default.nix or foo.nix)
  • Having the plugin already in its own directory makes future expansion easier: plugins/by-name: init #2164 (comment)
  • This simplifies the implementation; we only need to handle type == "directory" files.
• Some categories should remain
  • e.g. where we build many plugin modules with a specialized helper, such as mkLsp or mkCmpPlugin
  • I have no opinion on whether these should be in or out of by-name, but I have no immediate plans to move them.
  • Plugins outside of by-name should be imported manually in plugins/default.nix.

[khaneliman]

My opinion was that we'd just do a migration over to the new structure right away.

[MattSturgeon]

My opinion was that we'd just do a migration over to the new structure right away.

Sure, but I'd like the rest of the design decisions discussed & agreed on before investing time into that.

[khaneliman]

Design decisions

• We don't need a prefix directory (like in nixpkgs) because we are dealing with a smaller scale

  • By prefix, I mean the "na" directory in pkgs/by-name/na/name/package.nix.
  • We can just have plugins/by-name/<name>/default.nix to keep things simple.
  • Nixpkgs has around 500 prefixes alone in pkgs/by-name, while we have around 200 files total matching plugins/*/*.nix.
  • Having a prefix level would complicate both the implementation and usage, e.g. actually searching for a plugin's module.
  • Tab completion for plugins/by-name/examp<TAB> should be easier without a prefix.

I think this makes sense, we don't have a large enough scale project to warrant much (if any) nesting. The only thing I could see is maybe doing alphabetical nesting on first character if we were really worried about too large of a directory, for whatever reason.

• We shouldn't need to add by-name plugins to an imports list

  • It is trivial to read the directory and fold that into a list of paths to import.
  • This is one less thing for new contributors to worry about when adding a plugin module.

• We should exclusively use directories with default.nix files.

  • This adds consistency.
  • It is one less judgement call to make on new plugins (i.e. whether to use foo/default.nix or foo.nix)
  • This simplifies the implementation; we only need to handle type == "directory" files.

Completely agree, I think we talked about this previously, but it will make it consistent in finding a plugin and not have to worry about sorting when simple plugins are still using directories instead of a single file.

[MattSturgeon]

Ok, I've included an initial round of migration, just targeting the "utils" category for now.

Details

# fish

set category utils

# First, move the loose files
for f in plugins/$category/*.nix
    set name (path change-extension '' $f | path basename)
    set dest plugins/by-name/$name
    mkdir -p $dest
    git mv $f $dest/default.nix
end

# next, move the remaining dirs over
git mv plugins/$category/* plugins/by-name/

# Remove the old imports in `default.nix`
vim plugins/default.nix
git add plugins/default.nix

# We also need to move the test files, for consistency:
for f in tests/test-sources/plugins/$category/*.nix
    set name (path change-extension '' $f | path basename)
    mkdir -p tests/test-sources/plugins/by-name/$name
    git mv $f tests/test-sources/plugins/by-name/$name/default.nix
end
git mv tests/test-sources/plugins/$category/* tests/test-sources/plugins/by-name/

[khaneliman]

Yeah, initial impression is that directory layout should be fine. I'm curious what @traxys and @GaetanLepage think. I think utils was our largest directory of plugins and that being flattened into by-name isn't that large.

[traxys]

We should exclusively use directories with default.nix files.

Another advantage is that it allows to easily split the default.nix file in sub-nix files if need arises, wheras if you had <name>.nix you would need to create <name>/default.nix

[traxys]

I agree with all the design choices

[MattSturgeon]

How far should this PR go with migrating existing plugins to by-name?

We already have enough migrated to demonstrate it working, and I don't want to spend too long deciding which plugins should/should not be moved over.

[khaneliman]

How far should this PR go with migrating existing plugins to by-name?

We already have enough migrated to demonstrate it working, and I don't want to spend too long deciding which plugins should/should not be moved over.

If we're going to be leaving some behind out of by-name, I'd say wherever you feel comfortable stopping. The ones that get left behind we can decide if they should be migrated or not in separate discussions. My original thought was that it was gonna be an all out migration, but I do see the notes about some things not easily migrated, due to the shared code and helpers. So, I think it's fair to just knock out the bulk ones and niche ones can be done later, if it makes sense.

[khaneliman]

Future tooling

I think it'd be useful to have a devshell command that interactively builds a new plugin module based on the template.

You would be prompted for things like "type" (vim, lua, etc) "name", "pkg name" "setup fn", "globals prefix", and it'd automatically produce a minimal module file.

$ create-plugin
What's the plugin's name? 
> hello

What's the plugin type? (1. neovim (i.e. `require().setup()`) 2. vim with globals 3. vim without settings)
> 1

Current configuration:
1. name = "hello"
2. type = lua/neovim with setup function
3. luaName = "hello"
4. pkg = pkgs.vimPlugins.hello
5. setup = ".setup"

Would you like to change anything? (type a line number to modify it, or <return> to finish)
>

I think this is a great idea! The internal tooling and helpers have gotten to the point it should be fairly simple to generate a skeleton module.

[MattSturgeon]

This should be ready for review.

Note that @GaetanLepage has specifically asked to have a say before merging.

Ideally, I'd like approval from all maintainers on this one.

I appreciate it is a large PR, however the vast majority of changes are simple renames with no changes to the files being moved.
The actual changes are mostly in:

• CONTRIBUTING.md (updated docs)
• plugins/default.nix (removing imports)
• modules/plugins.nix (auto-import implementation)
• tests/plugin-by-name.nix (various tests)

Everything within plugins/* and tests/test-sources/ should exclusively be moving files without actual modification.

I've limited the migration to the low-hanging fruit. Partly for my own sanity, partly to avoid controversy/distraction, partly to keep the bulk part of the review simple.
(Reviewers would have to take more care if I migrated more complex plugins to by-name)

Hopefully the focus of the reviews can be on the design decisions, the extent of the tests (should they go further? additional tests?), any nit-picking with the implementation, and (of course) proof-reading the documentation.

cc @traxys @khaneliman

[MattSturgeon]

I've added very basic empty tests for the few plugins that didn't have any by running:

# fish
for name in autoclose commentary easyescape floaterm fugitive gitmessenger intellitab nvim-bqf nvim-colorizer surround treesitter-playground treesitter-refactor vim-matchup
    echo "adding test for $name"
    set dir tests/test-sources/plugins/by-name/$name
    mkdir -p $dir
    echo "{
      empty = {
        plugins.$name.enable = true;
      };
    }" > $dir/default.nix
    git add $dir/default.nix
end

With this, I felt comfortable adding a test to tests/plugins-by-name.nix that validates a default.nix file exists for all directories under plugins/by-name and a corresponding tests/test-sources file also exists.

[khaneliman]

LGTM, I look forward to the convenience of by-name

[MattSturgeon]

@Mergifyio queue

[mergify]

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 54599ad