lib/utils: add mkWarnings

[GaetanLepage]

This aims to provide more consistency for our warnings.

• lib/utils: add mkWarnings
• treewide: use mkWarnings wherever possible

Before:

warnings = lib.optional COND ''
  Nixvim (plugins.foo): MESSAGE
'';

After:

warnings = lib.nixvim.mkWarnings "plugins.foo" {
  condition = COND; # Or `cond = `, to be discussed
  message = "MESSAGE";
};

Notes:

• The second argument of mkWarnings can either be a single conditional warning or a list of several ones.
• A conditional warning is either a string (i.e. non-conditional) or a { condition = ...; message = ...; } attrs.

[khaneliman]

I like it just to be consistent treewide. Error is because you didn't inherit it in the lib/default.nix to get it available at top-level .

[GaetanLepage]

I like it just to be consistent treewide.

Yes I plan to do it in this very PR. Was just waiting for feedback first ;)

Error is because you didn't inherit it in the lib/default.nix to get it available at top-level .

Thanks, I'll change that.

[khaneliman]

I like it just to be consistent treewide.

Yes I plan to do it in this very PR. Was just waiting for feedback first ;)

Sorry, that was what I was trying to convey. I liked it to just get that consistency treewide without having to think about what our current warning formatting is and we only have to update it in one place to affect everywhere.

[GaetanLepage]

Sorry, that was what I was trying to convey. I liked it to just get that consistency treewide without having to think about what our current warning formatting is and we only have to update it in one place to affect everywhere.

Yes, I got this after answering !
This was the motivation for this addition.

[MattSturgeon]

Can we add a test to tests/lib-test.nix?

[MattSturgeon]

Q: should the condition be inverted so that it matches assertion-style logic?

I always get confused when assertions have negative conditions and warnings have positive conditions.

Although "normal" warnings having positive conditions while nixvim-warnings having negative conditions would also be confusing.

[GaetanLepage]

Yes, my opinion is to keep them positive.

[MattSturgeon]

One way we could solve this is by having two well-known condition attrs: e.g. expect and unless or when and whenNot, etc

Speaking of names, I think condition kinda implies "thing we expect to be true, warn when it is false"?

[khaneliman]

One way we could solve this is by having two well-known condition attrs: e.g. expect and unless or when and whenNot, etc

I kinda like this... if we're going to create our own helper might as well make it feel more robust. I think we either have it follow upstream if just a plain condition or allow defining both postive/negative cases.

[GaetanLepage]

I agree. I was planning to introduce a similar wrapper for assertions too.

[MattSturgeon]

Could do something like:

processCondition = warning: warning.when or !(warning.expect or false);

?

Or could be more robust to prevent typos:

processCondition =
  warning:
  if warning ? when then
    warning.when
  else if warning ? expect then
    !warning.expect
  else if buitins.isString warning then
    true
  else
    throw "warning is not a string and has no condition: ${toPretty { } warning}";

[MattSturgeon]

Before you get too carried away with the treewide changes:

We can consider using coercedTo in the actual warnings option type, something like:

type = listOf (coercedTo attrs mkWarning str);

This would prevent the need to have access to lib when defining warnings.

It may need to be a bit more complex than that, to allow for optional/conditional warning defs. So maybe we'd do this in another PR, if we even want to do it at all?