Optional simplified configuration syntax

[quentinmit]

See comments on #94 for more discussion.

The recent change to add a .value to every configFile setting made my configuration much more verbose.

This PR uses lib.types.coercedTo to support both old-style

programs.plasma.configFile.kwinrc.Xwayland.Scale = 1.25;

as well as the new

programs.plasma.configFile.kwinrc.Xwayland.Scale = {
  value = 1.25;
  immutable = true;
};

With this change, plasma-manager is now compatible with both old and new user configurations, aside from keys in nested groups. I added test cases to the flake check to make sure that's true.

It sounds like the behavior of nested groups is more controversial, so I'll split that off into a separate PR. This PR preserves the new syntax of apprc."foo/bar".field for nested groups, so any existing configs with nested groups are still incompatible with HEAD.

[magnouvean]

This looks good once we can get the checks to pass (it worked on my config just fine). I'm OK with these changes enough that I can merge this when this is fixed.

Continuing the discussions on the nested groups thing, a big problem was that it paired quite badly with the new format. For example the following config:

programs.plasma.configFile.somefile.group1.group2.group3.key = { value = 1; immutable = true;};

could be interpreted in two ways:

1. An immutable key "key" with value 1 in "[group1][group2][group3]" in "~/.config/somefile" (the intended way looking at the config)
2. Two normal keys "value" and "immutable" with values 1 and true in "[group1][group2][group3][key]" in "~/.config/somefile"

Additionally it made things more hard to debug to the point it hindered innovation in other parts of the project. This is a little different to the examples you brought up which are effectively just json and ini types (and those types support the recursive structures). There they are able to just write this to config directly without having to do much parsing (since the outputs there are json or ini). I didn't mean that the syntax itself was complicated, the syntax is probably better (I just don't think it's worth it in the grand scheme of the project). Seeing as I'm not going to be much involved in the project in the future, I'm not going to block any prs because of the maintainability argument though if there is significant disagreement here though.

[quentinmit]

This looks good once we can get the checks to pass (it worked on my config just fine). I'm OK with these changes enough that I can merge this when this is fixed.

Ack, I tested with nix flake check before I sent the PR, but I didn't know there were also two other flakes to test.

It looks like the one failing check is nix flake check -L --keep-going ./examples/systemFlake, and that's failing because it's got its own flake.lock that takes the trunk version of plasma-manager, and not the one in the PR. I just changed it to use ../.. and it appears to pass.

Continuing the discussions on the nested groups thing, a big problem was that it paired quite badly with the new format. For example the following config:

programs.plasma.configFile.somefile.group1.group2.group3.key = { value = 1; immutable = true;};

could be interpreted in two ways:

1. An immutable key "key" with value 1 in "[group1][group2][group3]" in "~/.config/somefile" (the intended way looking at the config)
2. Two normal keys "value" and "immutable" with values 1 and true in "[group1][group2][group3][key]" in "~/.config/somefile"

I think you can resolve this unambiguously by saying "If an attrset is a valid value, treat it as a value. Otherwise, expand it into an attrset of values." So in this example, it would be treated as case 1, and if you wanted case 2, you would need to set programs.plasma.configFile.somefile.group1.group2.group3.key.value.value = 1;. I agree that's clumsy, but it seems like a rare case to begin with.

Another rare case that I noticed is that it's technically possible for the same name to be used as both a key and a group. i.e.

[group1]
group2 = first value
[group1][group2]
key = second value

which wouldn't be possible with recursive definitions. IDK if that ever happens in practice.

Additionally it made things more hard to debug to the point it hindered innovation in other parts of the project. This is a little different to the examples you brought up which are effectively just json and ini types (and those types support the recursive structures). There they are able to just write this to config directly without having to do much parsing (since the outputs there are json or ini). I didn't mean that the syntax itself was complicated, the syntax is probably better (I just don't think it's worth it in the grand scheme of the project). Seeing as I'm not going to be much involved in the project in the future, I'm not going to block any prs because of the maintainability argument though if there is significant disagreement here though.

Speaking of "hard to debug", I'm actually working on another branch where I added a bunch more tests to test/basic.nix, and I found a number of other bugs in write_config.py. So I'm going to work on getting that to a reviewable state and send it as another PR (with no changes in the user-visible config). Then maybe I'll experiment with options for recursive definitions on a third branch.

[magnouvean]

Nice, I'll check if this works (though we should probably update the other example flakes as well then). Regarding recursively nested attrs, one thing to note is that I believe I have seen groups where one may want to set keys with the name "immutable" and maybe "value" and "persistence" as well, so it could be a somewhat rare, but still valid use-case which would have two interpretations for plasma-manager. As said, I personally don't think the current syntax is particularly bad, and not bad enough to introduce all these problems, but I won't stop you from experimenting of course (though if it would be merged isn't guaranteed).

I'm curious about the other pr which adds some tests, and if it's sent within a week or so I can probably review that and fix some additional bugs in the python script.

[quentinmit]

Nice, I'll check if this works (though we should probably update the other example flakes as well then). Regarding recursively nested attrs, one thing to note is that I believe I have seen groups where one may want to set keys with the name "immutable" and maybe "value" and "persistence" as well, so it could be a somewhat rare, but still valid use-case which would have two interpretations for plasma-manager. As said, I personally don't think the current syntax is particularly bad, and not bad enough to introduce all these problems, but I won't stop you from experimenting of course (though if it would be merged isn't guaranteed).

WDYT about removing flake.lock from both of the example flakes, and changing the CI to pass --override-input plasma-manager ../.. when it builds them? That would mean that every run of the CI would test against nixpkgs HEAD and the current PR's plasma-manager code, but if a user copies the flake as a template for themselves, they would generate a flake.lock with current references the first time they build it.

[magnouvean]

The flake.lock file isn't really the important part in the examples, I think the main reason for them being there is so that the flake check doesn't fail. I think this sounds like a good idea

[quentinmit]

The flake.lock file isn't really the important part in the examples, I think the main reason for them being there is so that the flake check doesn't fail. I think this sounds like a good idea

Alright, I just pushed that change.

[magnouvean]

Nice, this looks good now then, I'll merge once the CI completes.