Add module to manage konsole and it's profiles

[toast003]

No description provided.

[toast003]

So far the module can only set the default profile, I'm working rn on generating profiles

[toast003]

Sorry for the wait btw, things got in the way

[toast003]

I think #72 fixes the check that's failing, it shouldn't be my fault

[toast003]

Also @magnouvean I have noticed that after removing something from plasma.dataFile the file doesn't dissapear, it just stays there.
Is that intended or am I missing something?

[magnouvean]

If you are talking about keys I think that is what has always been the case (unless you use overrideConfig), and that you can only consistently unset a key if you set the value to null. As for the tests we could merge that soon I reckon and see if that fixes it. Your implementations look pretty good!

[m1-s]

I think #72 fixes the check that's failing, it shouldn't be my fault

Sorry that was my bad. I have included a fix in the mentioned PR.

[magnouvean]

It seems like it passes with the tests updated at least :)

[toast003]

Trying to add an assertion in case you set the font size without a name since konsole doesn't support that, but I can't get it to work.
Anyone mind helping me out?

This is what I tried:

config = mkIf (config.programs.plasma.enable && cfg.enable) {
  assertions = [
      {
        assertion = ((builtins.isInt cfg.font.size) && (builtins.isNull cfg.font.name));
        message = "You cannot change the font size without specifying a font!"; 
      }
  ];
  ...
}

[magnouvean]

Shouldn't it be the opposite, i.e. you assert that !((builtins.isInt cfg.font.size) && (builtins.isNull cfg.font.name)) or equivalently (cfg.font.name != null || !(builtins.isInt cfg.font.size)). That's the way normal nix assertions work at least.

[toast003]

Can't wrap my head about it, this is what I have so far:

diff --git a/modules/apps/konsole.nix b/modules/apps/konsole.nix
index 111592a..c9f3f9d 100644
--- a/modules/apps/konsole.nix
+++ b/modules/apps/konsole.nix
@@ -43,6 +43,14 @@ let
         };
       };
     };
+
+    # Some helper functions
+
+    # Takes a profile and returns true if there's no font set
+    isFontMissing = (value: builtins.isNull value.font.name);
+
+    # Takes a profile and returns true if there is a font size set
+    isFontSizeSet = (value: builtins.isInt value.font.size);
   };
 in
 
@@ -79,7 +87,18 @@ in
         }
       )
     ];
-
+    assertions = [
+        {
+          # assertion = !((builtins.isInt profilesSubmodule.font.size) && (builtins.isNull profilesSubmodule.font.name));
+          
+          assertion = (
+            lib.lists.any (
+              value: !((isFontMissing value) && (isFontSizeSet value))
+            ) (lib.attrsets.attrValues cfg.profiles)
+          );
+          message = "You cannot change the font size without specifying a font!"; 
+        }
+    ];
     # Konsole is fine with using symlinked profiles so I'll use the home-manager way
     programs.plasma.dataFile = mkIf (cfg.profiles != {}) (
       mkMerge ([

[magnouvean]

Maybe:

assertions = [
      {
        assertion = (
          lib.lists.any (
            (map (value: !((isFontMissing value) && (isFontSizeSet value))) (lib.attrsets.attrValues cfg.profiles))
        );
        message = "You cannot change the font size without specifying a font!"; 
      }
];

[toast003]

No, that doesn't seem to work either :(
For now I'll just set a default font, if I keep trying to figure out the assertion I'm just going to get burnt out

[magnouvean]

Sounds good, I may try to make a separate pr when I have time, but this can be merged before that anyway I think

[toast003]

Think this should be enough settings for most people
I'll see if I can clean up old profiles but apart from that this should be good to go

[magnouvean]

Maybe adding an option to configure general konsolerc settings would be nice as well. I think it would be better to have this built in here rather than to have to configure this separately via the lower-level settings. Could do a separate pr for it though, but it should be quite simple (just adding an option for it and adding to the mkMerge).

[toast003]

So something like the extraSettings options in most NixOS modules, right?
That should be easy to do

EDIT: I didn't add more options for konsole itself since I figured most of what people want to change is in the profiles, and not on konsole. That said if there is something that I didn't think about I can add that too

[toast003]

Also I have realized that I the format for the profiles is close enough to an INI file that I might be able to get away with using xdg.dataFile with generators.toINI instead of our own dataFile, which would clean up old entries. So I'll go ahead and switch to that

[magnouvean]

Yeah, basically the same as the home-manager options would be good.

[toast003]

Ok, this should be good enough for now, I'll mark it as ready for review

[magnouvean]

See my comments. I also saw that some of the descriptions are missing a period at the end. Other than that this looks good!

[toast003]

Oh also @magnouvean
In #94 (comment) you mention changing the format for config files. Since I copied a bit of code from files.nix so programs.konsole.extraConfig still uses the old format

[magnouvean]

Good to know. I reckon though that if you feel like this pr is good now it can be merged without thinking about #94, and I'll tweak this module in that pr (it should be easy)

[Svenum]

Looks good to me