modules/performance: add uncompiledPlugins option to plugin byte compilation

[saygo-png]

This option is a sister option to standalonePluginsin combinePlugins. It allows the user to skip some plugins from byte compilation.

This is a needed change because some plugins (I know of at least faster and conform) break when byte compiled. With this option users don't have to choose between having byte compilation, having broken functionality or not using certain plugins. It allows for a compromise where only the problematic plugins are skipped, meanwhile the rest is still compiled.

I've tested that addingfaster.nvim to the exception list (either as a package or name) fixes #3659

Things of note:

1. This breaks all existing configurations that set performance.byteCompileLua.plugins to a boolean.

   I could not figure out a way to deprecate this without an error, because this PR changes the old option to a set.
   The error thrown by nixpkgs is fairly good.

   • mkRenamedOptionModule throws infinite recursion
   • mkRemovedOptionModule throws because .plugins is not removed
   • mkChangedOptionModule throws because .plugins is not removed
   • an assertion in assertions is overridden by the nixpkgs assertion of the module system. It detects that the attrset is set to a boolean and throws a "type" error.

2. This PR does not include a test.

   The lua tests are broken due to a build failure for luajit. I couldn't feasibly write a new and correct test without the ability to run it and check its behavior.

[MattSturgeon]

1. I could not figure out a way to deprecate this without an error, because this PR changes the old option to a set.

One way we could convert a boolean option into a set of options would be to actually make the old boolean a union of (bool, submodule).

Using a submodule for this is a little over-kill, but it's currently the easiest way to encapsulate a set of options within an option's type.

The union could be oneOf, either, coercedTo, or nixvim's custom fork of coercedTo: transitionType (yes, I still need to upstream something like it to nixpkgs lib).

In this case it'd look something like:

plugins = lib.mkOption {
  type =
    let
      oldType = lib.types.bool;
      newType = lib.submodule {
        options = {
          enable = lib.mkEnableOption "plugins" // {
            description = "Whether to byte compile lua plugins.";
          };
          uncompiledPlugins = lib.mkOption {
            type = with types; listOf (either str package);
            default = [ ];
            example = [ "faster.nvim" ];
            description = "List of plugins (names or packages) to exclude from byte compilation.";
          };
        };
      };
    in
    lib.nixvim.transitionType oldType (enable: { inherit enable; }) newType;
  # ...
  # NOTE: don't declare a default here, to avoid it being documented...
  # Instead, define `plugins = lib.mkOptionDefault {}` in an unconditional config section.
}

At some point we'd want to simplify the submodule to a plain set of options, which is technically a breaking change. It's breaking because people can assign any kind of module to a submodule-type option, including a function, file, compact, or verbose style module. Including imports, additional option declarations, etc.

We could disallow that by replacing the type's check function with lib.isAttrs:

newType' = newType // {
  description = "attribute set";
  check = builtins.isAttrs;
};

...but the complexity is adding up just to cater to really niche deprecation scenarios. And it'd still allow { imports = []; } and { options = {}; and { config = {}; } (etc) style modules anyway; it'd only block files and functions.

Sadly, there isn't an easy alternative to submodules for encapsulating plain sets of options in a type, though. The closest proposal I'm aware of is lib.types.record, but even that is a bit fancier than just a set of options...

[MattSturgeon]

Interesting concept. Thanks again for another great contribution!

I haven't considered what use-cases this has (maybe @stasjok will have an opinion?).

In the meantime, here's some initial feedback on the implementation:

[stasjok]

I haven't considered what use-cases this has (maybe @stasjok will have an opinion?).

I don't have objections to the ability to exclude plugins from compilation. Though in this particular case with telescope I think this is better to be fixed upstream. But who knows, this option might be useful for some niche cases for someone.

[stasjok]

Maybe in order to preserve backward compatibility to leave plugins option as is and add another one like excludedPlugins or pluginExceptions? So it is defined like this:

      performance.byteCompileLua = {
        enable = true;
        plugins = true;
        excludedPlugins = [ ... ];
      };

I don't think this option will be used often. AFAIK byte compilation is pretty harmful, except when you need a plain text files or debug.* module.

There is also a -g flag to luajit that preserves debug info (if it's enabled, backtraces would point to original files and with correct line numbers). I didn't enable it in order to produce minimal files. But I though about leaving an option (it would be awkward because byte compilation helpers are in lib).

[saygo-png]

I've used the submodule approach, but i can't figure out a way of hiding the old option from docs. I have to include a description otherwise a warning is shown for a lack of a description. We have warnings to errors enabled on this so the build fails.

> error: option performance.byteCompileLua.plugins has no description
> Treating warnings as errors. Set documentation.nixos.options.warningsAreErrors to false to ignore these warnings.

Setting internal = true on plugins also hides the suboptions from docs. Setting internal = false on the suboptions does not help.

Is there a way around this or should I just use the approach suggested by Stasjok?

[MattSturgeon]

SGTM