nix-community / nixvim

Configure Neovim with Nix! [maintainer=@GaetanLepage, @traxys, @mattsturgeon, @khaneliman]
https://nix-community.github.io/nixvim
MIT License
1.65k stars 257 forks source link

Lualine: mixed tables are not allowed? #923

Closed shofel closed 9 months ago

shofel commented 9 months ago

Hello guys! Hope you're well :)

I've stuck a bit. Please guide me, and maybe I'll solve the problem. I'm ready to contribute making a PR

Plugin affected: Lualine Nixpkgs channel: unstable (irrelevant) Home-manager version: irrelevant

Description

Lualine allows to specify the components as mixed tables: {lualine_a = [ {'tabs', mode = 2} ]}

I want to express this with nixvim, and fail.

type of plugins.lualine.sections.lualine_a is Type: null or (list of (string or (submodule)))

Config

I tried this: { plugins.lualine.tabline.lualine_a = [ {"0" = "tabs"; mode = 2;} ];

Here is a repo with a minimal repro https://github.com/shofel/nixvim-mixed-lualine/blob/ca1790ec28cfffb7ef99c8273e5f1c6b118e6e09/flake.nix

GaetanLepage commented 9 months ago

Yes, this is indeed possible. You can (you have to actually) provide the section name using the name key. Here is an example.

Unfortunately, the documentation is not super very clear about that.

shofel commented 9 months ago

Thanks for pointing this out! The tests are useful source of examples, indeed.

Also it works with the "0" as a key: { plugins.lualine.tabline.lualine_a = [{"0" = "tabs"; mode = 2;}]; };

It appeared, in my repro I missed the trailing ; in the "mixed" attrs object :see_no_evil:

I would suggests some ways to improve discoverability here. If any of them look good, then I could implement them

improvement 1

Add a link to the tests to the documentation of a plugin? Right after the Declared by: field. It looks too verbose though.. :thinking:

improvement 2

Bug report template: provide a guide how to write a failing test. How to run and write tests

improvement 3

Specify examples when declaring an option.
It will probably require to make (or tune) a function which creates types for plugin options

improvement 4

In readme.md: add an example of using a mixed table

shofel commented 9 months ago

improvement 5

Clarify that submodule actually means "attribute set"
I mean, in the type null or (list of (string or (submodule)))

GaetanLepage commented 8 months ago

Thank you for those ideas ! Here are some comments

improvement 1

Add a link to the tests to the documentation of a plugin? Right after the Declared by: field. It looks too verbose though.. 🤔

Yes, that will be too verbose I think. Ideally, documentation should be self-sufficient. I agree that it is not the case right now though.

improvement 2

Bug report template: provide a guide how to write a failing test. How to run and write tests

The issue here is that it would imply cloning nixvim or at least creating a nixvim flake to test the specific thing. Most of the time, when we have bug reports, we are able to understand what is wrong from what the user reports, even though it comes from a full configuration. I prefer people not having to worry to submit bug reports and thus lowering the barrier of opening an issue.

improvement 3

Specify examples when declaring an option. It will probably require to make (or tune) a function which creates types for plugin options

For conventional types, this is not helpful (it is trivial that possible values for a bool option are true and false). However, I do agree that examples should absolutely be added to more complex options so that the user directly knows how to set it. Also, the nixvim documentation is not meant to replace each plugin's documentation. Often, nixvim options are a 1 to 1 mapping of the plugin options, and thus, the user should refer to upstream doc to know how to use them.

improvement 4

In readme.md: add an example of using a mixed table

I didn't know that using the "0" key would work. The expected way to create lua mixed tables is prefixing the keys by __unkeyed:

{
  foo = 42;
  __unkeyed = 1;
  __unkeyedSomething = "hello";
};

would give in lua:

{
  ["foo"] = 42,
  1,
  "hello",
}

improvement 5

Clarify that submodule actually means "attribute set" I mean, in the type null or (list of (string or (submodule)))

This is a weakness of the auto-generated documentation. It doesn't dive into those submodule definitions. What we should do in those cases is provide more details (and example) in the documentation.

Conclusion

I don't want to sound like I reject any of those proposition. However, the solution for this lies mostly in documentation. Also, I plan to add links to user's configurations to the documentation so that people can see real world examples. Thank you once again for taking the time to write those suggestions. Any help with documentation is obviously welcome.

Happy hacking !

shofel commented 8 months ago

Thanks for taking time to think and answer!

Preface

The main point of this post is to think of a way to reduce tension between lua and nix
for both developers who add new plugins to nixvim, and for users who uses nixvim modules to configure vim plugins.

Text

I didn't know that using the "0" key would work. The expected way to create lua mixed tables is prefixing the keys by __unkeyed: Oh, it actually doesn't. Sometimes I'm really bad in manual testing :see_no_evil: :ghost:

Also, the nixvim documentation is not meant to replace each plugin's documentation. Often, nixvim options are a 1 to 1 mapping of the plugin options, and thus, the user should refer to upstream doc to know how to use them.

Often, nixvim options are a 1 to 1 mapping of the plugin options

Yes. And what if we could always have exact 1 to 1 mapping? So that the original docs of the plugins would always be relevant for nixvim (given both are up-to-date). I'll try to argue that it's possible, and then evaluate if it's worth to be done.

Here, with lualine options, we have two problems:

  1. The shape of objects differ in two ways: 1.1. In lua we have a mixed table, but in nix object we have the name attribute 1.2. In lua we have a top-level attributes (e.g. mode), but in nix we wrap them in extraConfig
  2. The second difference also leads to lack of type-checking. (While type-checking, as I believe, is a major excuse of extra-work for users caused by mapping from lua to nix)

A dream on a solution

Let's not evaluate cost for now, but just dream on an ideal solution.

What if we had a format of describing a plugin's options, which

  1. is a formal representation of the official plugin documentation
  2. can express any possible lua value (mixed tables are the only problem?)
  3. can be naturally converted to a piece of documentation. Let's say, a nested table where each option has its name, type and description. "Nested" means some cells contain a table. How to render those nesting tables is another challenge.
  4. can be converted to nix types for the module options

A challenge: mixed tables

There is a common way to express mixed tables for a nix module option. All the modules follow that way, and do not introduce their own ad hoc conversions.

Possible ways

Let's try with the following lua table:

    {
      'searchcount',
      maxcount = 999,
    }

The __unnamed key

Inspired by the current implementation in nixvim.

The name of the key? May be it is to be named `__unkeyed`, but I mistakenly recalled it as `__unnamed`. May be because it's a more natural name, but may be just by accident.
    {
      __unnamed = ["searchcount"];
      maxcount = 999;
    }

A function of a list and an attribute set

A mixed table mixes up a sequential table and a dictionary. Let's express this idea:

    utils.mixedTable(["searchcount"], {maxcount = 999})

A challenge: tagged unions

Lualine has different types of unions, depending on the head of a list:

    {
      'datetime',
      -- options: default, us, uk, iso, or your own format string ("%H:%M", etc..)
      style = 'default'
    }
    {
      'searchcount',
      maxcount = 999,
      timeout = 500,
    }

I believe this difference can't be expressed with nix types. But this kind of check can be implemented with pattern matching in nix runtime (attrsets.matchAttrs).

What are the possible downsides of giving up on nix types here?

There will also be a syntax in the option description language, and code of those runtime checks will be automatically generated.

Other challenges

Other plugins are to be checked out to find other rough edges

Conclusion

Are there any other unsolvable obstacles?

And does it look feasible at all? I would dive into this rabbit hole.

Here is a very broad prospect of work to be done:

GaetanLepage commented 8 months ago

Hey, Thank you very much for your interest and your detailed post. I really welcome the ambition and the willingness to make the project better.

However, I would have two reasons against such an important change:

With that said, I am still more than open to discuss and review any attempt at what you plan to implement. I really do not want to sound too conservative or negative about your proposition. I simply want to make very clear that there is a non-zero probability that we (I am not the only person in power here) refuse to merge such a thing. I would not want you to spend a ton of hours "for nothing". Hence, I would encourage you to start by a POC to see what it could look like.