stylix icon indicating copy to clipboard operation
stylix copied to clipboard

stylix: allow for safe config access in mkTarget

Open 0xda157 opened this issue 7 months ago • 4 comments

allows for accessing config once config has been removed from module args (if that ends up happening). Also allows for removal of the review burden of ensuring config.stylix or config.stylix.lib.colors isn't accessed when using config.

cc @trueNAHO @MattSturgeon @Flameopathic

Things done

Notify maintainers

0xda157 avatar May 23 '25 15:05 0xda157

IMO this feels like something that should be done in the file's top-level args (which are currently the module args), rather than configElement args.

Only being able to access config from within a configElement feels cumbersome; you may wish to also access it in option defaults, in shared/common let binding, etc.

Agreed. Though I can somewhat see the draw of wanting to eliminate the top-level args, I think that the visual improvement is very little in comparison to the increased difficulty for new contributors. Its abstractions are great for the future and people who understand it well, but the more mkTarget separates targets from the module system, the more difficult things will be to learn, understand, and debug.

Flameopathic avatar May 24 '25 12:05 Flameopathic

IMO this feels like something that should be done in the file's top-level args (which are currently the module args), rather than configElement args.

Only being able to access config from within a configElement feels cumbersome; you may wish to also access it in option defaults, in shared/common let binding, etc.

there are 3 options here

  1. access config through top-level args. I don't like this because we can get everything except colors, fonts, etc through top-level args, so below still applies.

Only being able to access config from within a configElement feels cumbersome; you may wish to also access it in option defaults, in shared/common let binding, etc.

  1. access config, colors, fonts, etc through top-level args. This breaks the mkIf != null thing.
  2. don't access config through top-level args

0xda157 avatar May 25 '25 18:05 0xda157

we can't get everything except colors, fonts, etc through top-level args, so below still applies.

What do you mean by this?

Flameopathic avatar May 26 '25 01:05 Flameopathic

I don't like this because we can't get everything except colors, fonts, etc through top-level args, so below still applies

I don't think that's necessarily true. With some refactoring, mkTarget could apply top-level args to its input argument using the fnOrAttrs pattern.

This would work the same way as (e.g.) nixpkgs override functions that accept either attrs or a function. This is also what the module system checks internally when importing modules, since they are also fnOrAttrs values.


With the current design, usage would be a bit ugly, but this would improve once modules are no longer modules and are instead implicitly "args for mkTarget".

{ mkTarget, ... }:
mkTarget (
  { config, options, pkgs ... }:
  {
    name = "foo";
    humanName = "bar";
  }
)

MattSturgeon avatar May 26 '25 01:05 MattSturgeon

either need to manual merge or have @danth approve for the auto merge to work

0xda157 avatar Jun 26 '25 01:06 0xda157

Successfully created backport PR for release-25.05:

  • #1592

stylix-automation[bot] avatar Jul 04 '25 17:07 stylix-automation[bot]