Add multiple accounts support to himalaya-watch

[jalil-salame]

Description

Currently services.himalaya-watch only supports selecting a single account to watch for changes, I have multiple accounts using himalaya and I would like to watch all of them.

I tried my hand at a patch, but the way I set it up you cannot replicate the behavior of choosing the primary account by default (you need to specify all by name):

himalaya-watch patch

 modules/programs/himalaya.nix | 55 ++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 27 deletions(-)

diff --git a/modules/programs/himalaya.nix b/modules/programs/himalaya.nix
index 2d216c3d..afad4a18 100644
--- a/modules/programs/himalaya.nix
+++ b/modules/programs/himalaya.nix
@@ -131,10 +131,10 @@ in {
         '';
       };
 
-      settings.account = lib.mkOption {
-        type = with lib.types; nullOr str;
-        default = null;
-        example = "personal";
+      settings.accounts = lib.mkOption {
+        type = with lib.types; listOf str;
+        default = [ ];
+        example = [ "personal" ];
         description = ''
           Name of the account the watcher should be started for.
           If no account is given, the default one is used.
@@ -171,31 +171,32 @@ in {
       globalConfig = compactAttrs himalaya.settings;
       allConfig = globalConfig // { accounts = accountsConfig; };
     in tomlFormat.generate "himalaya-config.toml" allConfig;
-    systemd.user.services = let
-      inherit (config.services.himalaya-watch) enable environment settings;
-      optionalArg = key:
-        if (key ? settings && !isNull settings."${key}") then
-          [ "--${key} ${settings."${key}"}" ]
-        else
-          [ ];
-    in {
-      himalaya-watch = lib.mkIf enable {
-        Unit = {
-          Description = "Email client Himalaya CLI envelopes watcher service";
-          After = [ "network.target" ];
-        };
+    systemd.user.services =
+      let inherit (config.services.himalaya-watch) enable environment settings;
+      in lib.genAttrs
+      (builtins.map (account: "himalaya-watch@${account}") settings.accounts) {
         Install = { WantedBy = [ "default.target" ]; };
-        Service = {
-          ExecStart = lib.concatStringsSep " "
-            ([ "${himalaya.package}/bin/himalaya" "envelopes" "watch" ]
-              ++ optionalArg "account");
-          ExecSearchPath = "/bin";
-          Environment =
-            lib.mapAttrsToList (key: val: "${key}=${val}") environment;
-          Restart = "always";
-          RestartSec = 10;
+      } // {
+        "himalaya-watch@" = lib.mkIf enable {
+          Unit = {
+            Description = "Email client Himalaya CLI envelopes watcher service";
+            After = [ "network.target" ];
+          };
+          Service = {
+            ExecStart = lib.concatStringsSep " " [
+              "${himalaya.package}/bin/himalaya"
+              "envelopes"
+              "watch"
+              "--account"
+              "%i"
+            ];
+            ExecSearchPath = "/bin";
+            Environment =
+              lib.mapAttrsToList (key: val: "${key}=${val}") environment;
+            Restart = "always";
+            RestartSec = 10;
+          };
         };
       };
-    };
   };
 }
-- 
2.43.0

This would remove the need for the enable option too (it would be enabled if accounts is non-empty). Currently it uses template unit, so the enable option does allow for people to dynamically enable more accounts using systemctl --user enable --now himalaya-watch@$account.service, but I am thinking it should be better to directly write the services instead of using a template unit.

Another thing I am considering is how to escape characters; right now my accounts are named after the email address I use (i.e. [email protected]) and if I remember correctly, systemd wants those characters (. and @) to be escaped and hex encoded (himalaya-watch@jalil\x40example\x46org.service).

[jalil-salame]

@soywod you probably want to add such a template service to himalaya itself. I would be glad to contribute it c:

[soywod]

Currently services.himalaya-watch only supports selecting a single account to watch for changes, I have multiple accounts using himalaya and I would like to watch all of them.

I thought about this last time, it is a very good point!

I tried my hand at a patch, but the way I set it up you cannot replicate the behavior of choosing the primary account by default (you need to specify all by name)

You can name the default account with the special string "default", so technically you can have a unit himalaya-watch@default. But we can also create a dedicated non-templated service for the default account.

but I am thinking it should be better to directly write the services instead of using a template unit

I like your idea of template unit, it fits well the need in my opinion.

Another thing I am considering is how to escape characters

This may be a pain point. What about letting users mapping their own names? Instead of settings.accounts being a list, we could have a set where the key would be the account name, and the value the template unit name?

@soywod you probably want to add such a template service to himalaya itself

In which form do you see this addition? As a template file in /assets directory? As a documentation entry in the FAQ?

[jalil-salame]

You can name the default account with the special string "default", so technically you can have a unit himalaya-watch@default. But we can also create a dedicated non-templated service for the default account.

O.O Then I'll add that to the module's documentation.

This may be a pain point. What about letting users mapping their own names? Instead of settings.accounts being a list, we could have a set where the key would be the account name, and the value the template unit name?

That would work if we are not using a systemd template, the template passes everthing after the @ to the unit (i.e. [email protected] will replace %i with personal. I am using --account %i so it is required for it to be the account name (which is why I thought about not using the template service). Another option is to just warn against using special characters in the account names c:

In which form do you see this addition? As a template file in /assets directory? As a documentation entry in the FAQ?

In assets so that other packages can make use of it benefiting people not using NixOS.

[soywod]

I am using --account %i so it is required for it to be the account name (which is why I thought about not using the template service).

I understand better. How do you imagine the version without template?

In assets so that other packages can make use of it benefiting people not using NixOS.

It makes sense, feel free to send a git patch whenever you want :)

[jalil-salame]

I am using --account %i so it is required for it to be the account name (which is why I thought about not using the template service).

I understand better. How do you imagine the version without template?

I imagine enumerating the accounts and creating a unit for each one of them.

In assets so that other packages can make use of it benefiting people not using NixOS.

It makes sense, feel free to send a git patch whenever you want :)

Already did c:

[soywod]

I imagine enumerating the accounts and creating a unit for each one of them.

But you would not have the same issue with the unit names?

Already did c:

How fast :D thanks a lot!

[jalil-salame]

I imagine enumerating the accounts and creating a unit for each one of them.

But you would not have the same issue with the unit names?

I was planning to assing them sequential numbers:

himalaya-watch-1.service, etc...

It's not very nice, but it fixes the issue...

[jalil-salame]

Just did a quick search, and there is systemd-escape.

You can do systemd-escape [email protected] $account and it will return the escaped unit name.

I don't know how to hook it into nix, but it should probably be added to the himalaya docs if you ship the template service.

[soywod]

Just did a quick search, and there is systemd-escape.

So great!

I don't know how to hook it into nix, […]

Good point, I don't see how to call this program from within Nix code. I tried to search for modules in nixpkgs solving this issue but I did not find anything relevant.

I have 2 ideas coming to my mind:

1. We let the user setting up the unit name in a set "{ account-name = unit-name; }", this way we can build multiple services "himalaya-watch..service",

or,

The "unit name prefix" must consist of one or more valid characters (ASCII letters, digits, ":", "-", "_", ".", and "")

2. we develop our own Nix function to escape unit names, for example replacing all invalid chars by "-". This way we can use systemd templates. We can just explain in the doc that unit names are kebab-case-like version of account names.

but it should probably be added to the himalaya docs if you ship the template service.

Totally agree, we can even add an example with systemd-escape.

[soywod]

The new v1.0.0-beta.4 is out, so it's the right moment to update the home manager package. I open a PR with your patch modified (https://github.com/nix-community/home-manager/pull/5297). I found the missing pieces:

• %I: Same as "%i", but with escaping undone. [doc]
• If you use the service with a non-escaped name, systemd do it for you with a warning:

  $ systemctl --user status himalaya-watch@bad~name
  Invalid unit name "himalaya-watch@bad~name" escaped as "himalaya-watch@bad\x7ename" (maybe you should use systemd-escape?).
  ○ himalaya-watch@bad\x7ename.service - Email client Himalaya CLI envelopes watcher service
       Loaded: loaded (/home/soywod/.config/systemd/user/[email protected]; disabled; preset: enabled)
       Active: inactive (dead)
  

I would love your feedback.

I open the issue as draft, as I also plan to integrate xdg.desktopEntries.

[jalil-salame]

I would love your feedback.

I open the issue as draft, as I also plan to integrate xdg.desktopEntries.

I'm struggling to find time, but I'll take a look when I can!