Skip to content

Conversation

@sand4rt
Copy link

@sand4rt sand4rt commented Dec 5, 2025


@stylix-automation stylix-automation bot added topic: home-manager Home Manager target topic: modules /modules/ subsystem topic: stylix /stylix/ subsystem labels Dec 5, 2025
@sand4rt
Copy link
Author

sand4rt commented Dec 5, 2025

Any idea why ci fails?

@0xda157 0xda157 changed the title feat: initial swayimg swayimg: init Dec 6, 2025
Copy link
Contributor

@0xda157 0xda157 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any idea why ci fails?

the pre-commit hooks are failing. you can avoid this happening in the future by running nix develop which will install the pre-commit hooks on your local git repo.


adding a testbed for this module is highly recommended, so pr reviews can test it themselves and the ci will reflect any configurations issues in the nixpkgs' module system.

humanName = "swayimg";

configElements =
{ colors, opacity }:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally colors and opacity would function independently of each other. the helix and ncspot modules are good examples of this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the pointers I'm not sure about how i should implement this, im fairly new to nix/stylix. should i add a extra option like;

  extraOptions = {
    background = lib.mkOption {
      type = lib.types.nullOr lib.types.str;
      description = "Used to set bg even if `opacity` or `colors` is null.";
      internal = true;
      default = null;
    };
  };

and then adjust the bg's like?;

gallery = {
    background = lib.mkIf (opacity.applications != 1.0) "#00000000";
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pattern is basically that internal options are used to share context across configElements function boundaries.

I suppose the base00 + swayimgOpacity expression should behave as follows:

colors opacity Expression
disabled disabled undeclared
disabled enabled undeclared
enabled disabled base00
enabled enabled base00 + swayimgOpacity

In that case, you would have something like:

extraOptions.opacity = lib.mkOption {
  type = lib.types.str;
  description = /* ... */;
  internal = true;
  default = "";
};

configElements = [
  (
    { cfg, colors }: {
      programs.swayimg.settings.viewer.window = base00 + cfg.opacity;
    }
  )

  ({ cfg, opacity }: { cfg.opacity = /* swayimgOpacity */; })
];

@stylix-automation stylix-automation bot added the topic: testbed Testbed changes label Dec 6, 2025
githubId = 87434959;
};
sand4rt = {
email = "[email protected]";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you do not wish to publish your email, it might be simpler to not include this optional field:

Suggested change
email = "[email protected]";

This line would also have to be removed in /generated/all-maintainers.nix.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose the following declarations are black for the same reason we have commit ccb411c ("mpv: set background to black (#1008)"):

Why is the transparency once 00 and once ff?

Unsure how I feel about hard-coding #000000d0. Would it not be better to set this one to base00?

in
{
environment = {
loginShellInit = "${lib.getExe package} flake-parts/flake.nix";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opening a Nix file with an image viewer is probably wrong. What about opening an actual image:

text = "swayimg ${
  (pkgs.callPackages ../../../stylix/testbed/images.nix {}).${
    config.stylix.polarity
  }
}";

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The testbed should enable the underlying Home Manager module of our /modules/swayimg/hm.nix module. Take a look at how other testbeds achieve this.

with colors.withHashtag;
let
swayimgOpacity = lib.toHexString (
((builtins.floor (opacity.applications * 100 + 0.5)) * 255) / 100
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These parentheses should be optional:

Suggested change
((builtins.floor (opacity.applications * 100 + 0.5)) * 255) / 100
builtins.floor (opacity.applications * 100 + 0.5) * 255 / 100

Comment on lines +34 to +39
font =
with config.stylix.fonts.monospace;
with config.stylix.fonts.sizes;
{
inherit name;
size = applications;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should go into a new configElements function that takes fonts as argument.

humanName = "swayimg";

configElements =
{ colors, opacity }:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pattern is basically that internal options are used to share context across configElements function boundaries.

I suppose the base00 + swayimgOpacity expression should behave as follows:

colors opacity Expression
disabled disabled undeclared
disabled enabled undeclared
enabled disabled base00
enabled enabled base00 + swayimgOpacity

In that case, you would have something like:

extraOptions.opacity = lib.mkOption {
  type = lib.types.str;
  description = /* ... */;
  internal = true;
  default = "";
};

configElements = [
  (
    { cfg, colors }: {
      programs.swayimg.settings.viewer.window = base00 + cfg.opacity;
    }
  )

  ({ cfg, opacity }: { cfg.opacity = /* swayimgOpacity */; })
];

@stylix-automation stylix-automation bot added the status: merge conflict Merge conflict label Dec 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: merge conflict Merge conflict topic: home-manager Home Manager target topic: modules /modules/ subsystem topic: stylix /stylix/ subsystem topic: testbed Testbed changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants