stylix: add addEnableOption to mkTarget

[0xda157]

required for gnome-text-editor to be moved to mkTarget cc: @trueNAHO @MattSturgeon @Flameopathic

Things done

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

Notify maintainers

[danth]

This feels out of place since the rest of mkTarget depends on that option existing, if this isn't a common scenario then we could just leave gnome-text-editor without mkTarget for now? What's the situation with other targets that use an overlay?

[MattSturgeon]

required for gnome-text-editor to be moved to mkTarget

That module is reading config.stylix.targets.gnome-text-editor.enable, which oddly is declared by it's overlay module... https://github.com/nix-community/stylix/blob/230705d5fb6c308402579b17e0261e9f15de6f46/modules/gnome-text-editor/overlay.nix#L9

This feels out of place since the rest of mkTarget depends on that option existing.

I agree, a target not having an enable option is wrong, by extension a target relying on some module elsewhere to declare its enable option feels like an anti-pattern.

In this case, can gnome-text-editor be refactored so that its enable option is not declared by its overlay module?

[0xda157]

In this case, can gnome-text-editor be refactored so that its enable option is not declared by its overlay module?

yes, but it would either require adding a nixos module for it or removing nixos support.

This feels out of place since the rest of mkTarget depends on that option existing, if this isn't a common scenario then we could just leave gnome-text-editor without mkTarget for now? What's the situation with other targets that use an overlay?

The issue with not using mkTarget is we have discussed changing hm/nixos/darwin/droid.nix to just be an arg to mkTarget instead of an actual module.

That module is reading config.stylix.targets.gnome-text-editor.enable, which oddly is declared by it's overlay module...

Because we want the overlay on exist on all platforms, without declaring it in the overlay.nix we would have to add a module for each platform.

[MattSturgeon]

This feels like it's more of an issue with the overall design of having separate platform & overlay modules, with no place for common shared modules.

Surely other targets have the same issue of "my hm module needs an enable option, as does my nixos module, as does my overlay module"?

It feels like bad design, but have you tried declaring the enable option from both the hm & overlay modules? IIRC the module system will try to merge option declarations, so declaring two identical options might be a no-op.

[0xda157]

This feels like it's more of an issue with the overall design of having separate platform & overlay modules, with no place for common shared modules.

Surely other targets have the same issue of "my hm module needs an enable option, as does my nixos module, as does my overlay module"?

I believe gnome-text-editor is only module with an overlay.nix and a home-manager module.

It feels like bad design, but have you tried declaring the enable option from both the hm & overlay modules? IIRC the module system will try to merge option declarations, so declaring two identical options might be a no-op.

it will not

error: The option `stylix.targets.gnome-text-editor.enable' in `/nix/store/3mfy153dqwmi427f9qwrzmnvmyxcz3r3-source/modules/gnome-text-editor/hm.nix' 
       is already declared in `/nix/store/3mfy153dqwmi427f9qwrzmnvmyxcz3r3-source/modules/gnome-text-editor/overlay.nix'.

[MattSturgeon]

I believe gnome-text-editor is only module with an overlay.nix and a home-manager module.

Relying on this assumption feels like it won't scale well.

I think we should use this module as an opportunity to come up with a better overall design, since it seems likely you'll run into the same issue again, at some point down the line.

[trueNAHO]

This feels like it's more of an issue with the overall design of having separate platform & overlay modules, with no place for common shared modules.

Surely other targets have the same issue of "my hm module needs an enable option, as does my nixos module, as does my overlay module"?

It feels like bad design, but have you tried declaring the enable option from both the hm & overlay modules? IIRC the module system will try to merge option declarations, so declaring two identical options might be a no-op.

The proposal from https://github.com/nix-community/stylix/issues/1230 might be relevant.

This feels out of place since the rest of mkTarget depends on that option existing, if this isn't a common scenario then we could just leave gnome-text-editor without mkTarget for now? What's the situation with other targets that use an overlay?

Why can we not move the

https://github.com/nix-community/stylix/blob/b69e9b761ee682b722e2c9ce46637e767b50f6dc/modules/gnome-text-editor/overlay.nix#L9-L10

declaration into /modules/gnome-text-editor/hm.nix and let it be generated by mkTarget? If /modules/gnome-text-editor/overlay.nix is causing problems by not being properly guarded anymore, could we not update our core overlay logic to take the /modules/<MODULE>/<PLATFORM>.nix guard into consideration?

I suppose this is what https://github.com/nix-community/stylix/pull/1384 tries to do.

[Flameopathic]

Can this be closed? I do not see a use case for it anymore.