home-manager
home-manager copied to clipboard
autorandr: configModule.extraConfig
Description
Add an option to programs.autorandr's configModule to allow arbitrary extra config lines.
No option exists for adding arbitrary key/values to generated autorandr profile config, as is common in other nix modules. This commit adds one.
Checklist
-
[x] Change is backwards compatible.
-
[x] Code formatted with
./format. -
[x] Code tested through
nix-shell --pure tests -A run.allornix develop --ignore-environment .#allusing Flakes. -
[x] Test cases updated/added. See example.
-
[x] Commit messages are formatted like
{component}: {description} {long description}See CONTRIBUTING for more information and recent commit messages for examples.
-
If this PR adds a new module
- [ ] Added myself as module maintainer. See example.
Maintainer CC
@uvNikita
@uvNikita Hello no worries on this but just wanted to ping you if you have the time for a review. I need this option for some device specific config that is not included in the regular config options. I'm sure having extraConfig can be useful to others who need to add something to their config quick and dirty.
Thank you!
Makes sense to have possibility to add extra options.
If the options are always in the format of key value, maybe it makes sense to add settings option instead, in line with rfcs42?
You can see implementation example in other modules, e.g. https://github.com/nix-community/home-manager/blob/d7830d05421d0ced83a0f007900898bdcaf2a2ca/modules/programs/i3status.nix#L30
@uvNikita Good point. I am not sure of the config format exactly for autorandr, but I can investigate! If it is strictly keys/values I can try to make settings. But if it turns out more complicated, or if it also allows flag type options where it's just a key and no value, then would extraConfig be fine then?
@zackattackz I think so. Thank you for looking into this and sorry for a late review :)
Hi @uvNikita
Sorry for the lack of response here. I appreciate your review.
I looked into the config file format and it just seems to be a bespoke parser: https://github.com/phillipberndt/autorandr/blob/770dc86bec9d20bbe22b596aad9aa1520c966720/autorandr.py#L505
It can be either keys/values pairs or also just independent keys ("off" for example).
I think this can be represented as a settings type of attrsOf (either str bool)
so for example:
config.autorandr.profiles.${profile}.config.output1.settings = {
"someBoolConfig" = true;
};
config.autorandr.profiles.${profile}.config.output2.settings = {
"somekey" = "somevalue";
};
Which would generate config like:
output output1
someBoolConfig
output output2
somekey someval
I think this wouldnt be too hard to implement?
But honestly, because I couldn't find a solid definition of the config format, and because the autorandr code is kind of a lot to take in, I could be missing some considerations here.
I feel like extraConfig would be easier to keep and maintain in the long run, though I do understand settings is preferred generally.
If you'd like, I can implement settings as described above (or some other way you see fit).
Or I can amend with the suggestion you made for extraConfig to merge that instead?
Since it's a custom format, I don't mind if we keep your original suggestion of adding extraConfig option. Another one can also be added later if need arises.
OK! Just lmk if anything else is needed, no rush of course.
Sorry forgot about those suggestions. Added them in, just needed a slight difference from your last suggestion.
Amended with the requested changes 🫡
@rycee Do you mind merging this one? I can't merge until test succeed, but doesn't look like this is related?
Merged now!
Thanks for the review :)