{i3,sway}: export bar configuration through options

[Flameopathic]

Related: #1469

Possibly related: #415

Things done

• Tested locally
• Tested in testbed
• Commit message follows commit convention
• Fits style guide
• Respects license of any existing code used

Notify maintainers

@mightyiam @butzist @trueNAHO

[trueNAHO]

Would it be desirable to generally expose every target's configuration? In that case, mkTarget would generate the option with a consistent description. Unsure whether the configuration should be declared in the option as:

options.stylix.targets.${name}.config = lib.mkOption {
  default = /* configuration */;
  readOnly = true;
};

config = config.stylix.targets.${name}.config;

or in the configuration as:

options.stylix.targets.${name}.config = lib.mkOption {
  readOnly = true;
};

config.stylix.targets.${name}.config = /* configuration */;

On a side note, once the mkTarget migration is complete, we could trivially allow end-users to extend mkTarget's configElements:

options.stylix.targets.${name}.extraConfig = lib.mkOption {
  type = /* mkTarget's configElements */;
  description = /* Provide a link to mkTarget's source code documentation */;
  default = [];
};

config = lib.mkIf (config.stylix.enable && cfg.enable) (
  lib.mkMerge (
    lib.optional (generalConfig != null) (mkConfig generalConfig)
    ++ map mkConditionalConfig (
      (lib.toList configElements) ++ (lib.toList cfg.extraConfig)
    )
  )
);

This would allow grouping styling-related declarations under the stylix namespace:

stylix = {
  enable = true;

  targets."<TARGET>".extraConfig =
    { colors }:
    { programs."<TARGET>".settings.background = lib.mkForce colors.base01; };
};

[Flameopathic]

Would it be desirable to generally expose every target's configuration?

I don't think so; I can't think of a use case. These specific examples are solutions to somewhat uncommon problems, and they require a specific subset of the target's config output, rather than the whole thing.

[mightyiam]

This exportedFoo, is there precedence for this in the ecosystem? I'm not opposed to it. Seems alright to me.

[danth]

I do question what makes these modules any different than Firefox and VSCode. For that reason, I'm not sure if I would say that this is a full formalization, as these targets need their configurations manually merged while Firefox and VSCode use an option to decide the attribute paths.

There's nothing fundamentally different - I think it's just that these modules are very old and haven't had a lot of attention since. It should be possible to make these modules work the way Firefox and VSCode do, or the other way around, to settle on a standard.

Would it be desirable to generally expose every target's configuration?

In most cases this is already available by reading the options we set, unless the user has overridden it. But in that case they would probably be interested in reading their overridden version anyway.

[Flameopathic]

This exportedFoo, is there precedence for this in the ecosystem? I'm not opposed to it. Seems alright to me.

No, this would be setting a precedent.

---

I do question what makes these modules any different than Firefox and VSCode. For that reason, I'm not sure if I would say that this is a full formalization, as these targets need their configurations manually merged while Firefox and VSCode use an option to decide the attribute paths.

There's nothing fundamentally different - I think it's just that these modules are very old and haven't had a lot of attention since. It should be possible to make these modules work the way Firefox and VSCode do, or the other way around, to settle on a standard.

Alright. In that case, I think that this NixVim option is still necessary because its use is slightly different than the others, but all of the other targets should be set up like VSCode and Firefox. To do that, though, it would probably be worth having some sort of abstraction for setting up warnings and/or options.

[0xda157]

the naming of this pr/commit/options doesn't make much sense, config already means something in the nix module system. Renaming the options to some else would be best, e.g. exportedModule for the nixvim module.

[trueNAHO]

I do question what makes these modules any different than Firefox and VSCode. For that reason, I'm not sure if I would say that this is a full formalization, as these targets need their configurations manually merged while Firefox and VSCode use an option to decide the attribute paths.

There's nothing fundamentally different - I think it's just that these modules are very old and haven't had a lot of attention since. It should be possible to make these modules work the way Firefox and VSCode do, or the other way around, to settle on a standard.

Alright. In that case, I think that this NixVim option is still necessary because its use is slightly different than the others, but all of the other targets should be set up like VSCode and Firefox.

The NixVim config is exposed to support the standalone use case:

commit 0a4755b656c28043c52063384fc68bbb30913903
Author: Matt Sturgeon <matt@sturgeon.me.uk>
Date:   2024-06-05 03:10:32 +0100

    nixvim: expose config as read-only option

    Allow standalone nixvim users to take advantage of stylix by exposing
    the generated config as `config.lib.stylix.nixvim.config`.

    This can be passed to the nixvim derivation's `extend` function or used
    directly in a nixvim configuration.

The other modules touched in this PR expose sensible theming declarations to allow end-users to build upon them to write their own configuration. This pattern is inconsistent with declaring a sensible default configuration without any real customizing capabilities (aside from simply overriding it with lib.mkIf), as used in /modules/plymouth/theme-script.nix and potentially other (pending?) modules.

Should these declaration expositions and default configurations be handled differently or not? The /modules/plymouth/theme-script.nix case is inherently hard to generally extend to same extent the exposed Nix attribute sets are.

[Flameopathic]

The other modules touched in this PR expose sensible theming declarations to allow end-users to build upon them to write their own configuration.

I was under the impression that this is not the primary goal of exporting the configuration and that the actual goal is to give the user a relatively simple way of applying theming to attributes with user-set paths (ex. wayland.windowManager.sway.config.bars.*).

This pattern is inconsistent with declaring a sensible default configuration without any real customizing capabilities (aside from simply overriding it with lib.mkIf), as used in /modules/plymouth/theme-script.nix and potentially other (pending?) modules.

Agreed. Do you mean lib.mkForce rather than lib.mkIf?

Should these declaration expositions and default configurations be handled differently or not? The /modules/plymouth/theme-script.nix case is inherently hard to generally extend to same extent the exposed Nix attribute sets are.

I believe that this is another good reason to make these targets function like Firefox and VSCode, which would mean they would behave more like regular targets.

[mightyiam]

I was under the impression that this is not the primary goal of exporting the configuration and that the actual goal is to give the user a relatively simple way of applying theming to attributes with user-set paths

👍

Simply overriding with lib.mkForce is flexible enough I think.

Targets that can behave like Firefox probably should. All they need to know is some attribute name to apply to. It's the list case that can't quite work that way and forces us to provide a value for the user to wire up themselves.

[trueNAHO]

The other modules touched in this PR expose sensible theming declarations to allow end-users to build upon them to write their own configuration.

I was under the impression that this is not the primary goal of exporting the configuration and that the actual goal is to give the user a relatively simple way of applying theming to attributes with user-set paths (ex. wayland.windowManager.sway.config.bars.*).

Either way, extending the Stylix configuration seems to be the end result.

This pattern is inconsistent with declaring a sensible default configuration without any real customizing capabilities (aside from simply overriding it with lib.mkIf), as used in /modules/plymouth/theme-script.nix and potentially other (pending?) modules.

Agreed. Do you mean lib.mkForce rather than lib.mkIf?

Yes, I meant lib.mkForce.

Should these declaration expositions and default configurations be handled differently or not? The /modules/plymouth/theme-script.nix case is inherently hard to generally extend to same extent the exposed Nix attribute sets are.

I believe that this is another good reason to make these targets function like Firefox and VSCode, which would mean they would behave more like regular targets.

What exactly do you mean by making the targets from this PR like Firefox and VSCode? Do you mean providing extraOptions to be merged inside our internal configElements declaration? In that case, should we just already implement the generic extraConfig option:

On a side note, once the mkTarget migration is complete, we could trivially allow end-users to extend mkTarget's configElements:

options.stylix.targets.${name}.extraConfig = lib.mkOption {
  type = /* mkTarget's configElements */;
  description = /* Provide a link to mkTarget's source code documentation */;
  default = [];
};

config = lib.mkIf (config.stylix.enable && cfg.enable) (
  lib.mkMerge (
    lib.optional (generalConfig != null) (mkConfig generalConfig)
    ++ map mkConditionalConfig (
      (lib.toList configElements) ++ (lib.toList cfg.extraConfig)
    )
  )
);

This would allow grouping styling-related declarations under the stylix namespace:

stylix = {
  enable = true;

  targets."<TARGET>".extraConfig =
    { colors }:
    { programs."<TARGET>".settings.background = lib.mkForce colors.base01; };
};

-- #1502 (review)

[Flameopathic]

Should these declaration expositions and default configurations be handled differently or not? The /modules/plymouth/theme-script.nix case is inherently hard to generally extend to same extent the exposed Nix attribute sets are.

I believe that this is another good reason to make these targets function like Firefox and VSCode, which would mean they would behave more like regular targets.

What exactly do you mean by making the targets from this PR like Firefox and VSCode?

Firefox and VSCode use profileNames options to determine the options that they are supposed to be setting (programs.firefox.profiles.<<name>>) because otherwise Stylix would not know where to apply configuration. I believe that all of these targets (except for NixVim) are exporting their configuration so that the user can apply it to the appropriate attributes with arbitrary paths (xsession.windowManager.i3.config.bars.«name» in the case of i3's bar config export). These are two fundamentally different ways of accomplishing the same task, and I believe that we want to unify them.

[Flameopathic]

It's the list case that can't quite work that way and forces us to provide a value for the user to wire up themselves.

I completely missed this detail. I hadn't realized the difference between something.* and something.<name>.

I have adjusted the scope of this PR to exclude i3status-rust (as its bar configurations are named and can be treated like Firefox) and nixvim (as I have already made the necessary change in #1535).

[mightyiam]

I notice that this is marked as ready for review but just making sure by asking. Would it help if I tested this against my presonal usage at this point?

[Flameopathic]

This is ready for review, but it no longer includes NixVim. If you use i3 or sway bars, though, it would help if you tested this.

[mightyiam]

Ah, no, I don't use them, sorry. Thanks. Keep up the good work.

[trueNAHO]

For reference, although I have opened #1601, this should not affect the scope of this PR. The scope of this PR is good as-is.

[trueNAHO]

LGTM.

[stylix-automation]

Successfully created backport PR for release-25.05:

• [release-25.05] {i3,sway}: export bar configuration through options #1633