I don't particularly approve of the in-band signalling here. I'd rather have #82137's style of section generation.

I wasn't sure if there would be other sections with that type of notation. At the moment the nixos module only configures gerrit.config. I had in mind that it could also be used to generate the other config files. For example I have this one:

its/actions-its-jira.config

# https://gerrit.googlesource.com/plugins/its-base/+/master/src/main/resources/Documentation/config-rulebase-common.md
[rule "open"]
    event-type = patchset-created
    action = add-standard-comment
[rule "merged"]
    event-type = change-merged
    action = add-standard-comment
[rule "abandoned"]
    event-type = change-abandoned
    action = add-standard-comment
[rule "restored"]
    event-type = change-restored
    action = add-standard-comment

Note that I'm not complaining about the module options themselves — I'm quite happy with the new input format, and had been intending to refactor pluginSettings away myself. My problem here is with the in-band signalling inside the formatting code.

My bad, I am not familiar with the meaning of "in-band signalling". Did you mean that the comment should be moved up to the function definition? If yes then I agree :)

fixed

By "in-band signalling", I mean that data and framing information aren't kept separate — I should be able to express an attribute named "foo.bar" distinctly from a nested attribute set foo.bar. Just joining and splitting the strings without an escaping mechanism doesn't work for that. My prior implementation did not have this issue.

Hmm ok. I will take another look on monday then and make another PR

lazyAttrsOf should ideally only be used if either mkIf false definitions don't make sense (which isn't the case here), or if such definitions are handled with an emptyValue that then gets filtered out (e.g. by using lazyAttrsOf (nullOr ...), then filtering out null's before generating the file). Because this isn't done here, the following will error:

{
  services.gerrit.settings.foo.bar = mkIf false 10;
}

error: The option `services.gerrit.settings.foo.bar' is used but not defined.

Or alternatively just using attrsOf instead (which then doesn't support values to depend on other values, which is probably fine here)

So just replace all the lazyAttrsOf with attrsOf? I must admit that I cargo-culted this and don't have a good understanding of these distinctions even after your explanation.