Skip to content

i3status-rust: apply background opacity#1450

Merged
trueNAHO merged 1 commit intonix-community:masterfrom
mightyiam:i3status-rust-opacity
Jun 8, 2025
Merged

i3status-rust: apply background opacity#1450
trueNAHO merged 1 commit intonix-community:masterfrom
mightyiam:i3status-rust-opacity

Conversation

@mightyiam
Copy link
Copy Markdown
Member

I've been using i3status-rust via i3bar-river. Using the opacity implementation in #1415 results in the unused part of the bar having transparency but the part "used by" i3status-rust part has its own non-transparent backgrounds. This PR fixes that by adding opacity bits to i3status-rust. Note that it seems that i3bar-river must have blend = false (not sure whether that seems like inverse logic to me).

Have not tested with other bars that can render i3status-rust output. If it breaks for any other users we could revert and I will try a different approach. Is that an acceptable risk?

Things done

Notify maintainers

@stylix-automation stylix-automation bot added the topic: home-manager Home Manager target label Jun 3, 2025
@mightyiam mightyiam force-pushed the i3status-rust-opacity branch from eee0595 to c67bffd Compare June 3, 2025 14:53
@mightyiam mightyiam marked this pull request as ready for review June 3, 2025 15:00
@panchoh
Copy link
Copy Markdown
Contributor

panchoh commented Jun 6, 2025

Hi, @mightyiam. (big fan here, yay!)

Please consider reworking your PR to fit the new style recently introduced, by leveraging the mkTarget function.

For illustration, see the Module template section of the documentation, the doc embedded in stylix/mk-target.nix itself, and, for instance, this recently-refactored module, which is similar to this one.

This change was discussed here, and is being tirelessly applied throughout the tree (thanks, @awwpotato !!!).

@mightyiam mightyiam force-pushed the i3status-rust-opacity branch from c67bffd to 3afe114 Compare June 6, 2025 08:28
@mightyiam
Copy link
Copy Markdown
Member Author

I'm not sure whether this can fit into mkTarget usage. This module defines config.lib.stylix.i3status-rust.bar.

Documentation doesn't seem to refer to this specifically.

And I did not find any such examples. Perhaps this module should remain as is for the time being?

Semantically, this is a target but a "moving target". Perhaps support for those could be added later.

@panchoh
Copy link
Copy Markdown
Contributor

panchoh commented Jun 6, 2025

I'm not sure whether this can fit into mkTarget usage. This module defines config.lib.stylix.i3status-rust.bar.

Documentation doesn't seem to refer to this specifically.

And I did not find any such examples. Perhaps this module should remain as is for the time being?

Semantically, this is a target but a "moving target". Perhaps support for those could be added later.

Ah, you raise a valid point there. Disregard my previous comment, and let’s keep it as it is.

Copy link
Copy Markdown
Member

@danth danth left a comment

Choose a reason for hiding this comment

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

Code LGTM, although I haven't tested this personally.

And yes, this target is special because it provides a value under config.lib for people to use, rather than applying it directly to the configuration. It would be good to mention that in the meta.description so people know how to use the target (but that's not relevant to this PR)

Copy link
Copy Markdown
Member

@trueNAHO trueNAHO left a comment

Choose a reason for hiding this comment

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

Have not tested with other bars that can render i3status-rust output. If it breaks for any other users we could revert and I will try a different approach. Is that an acceptable risk?

Sounds good to me. Unfortunately, the module currently has no maintainers that could be asked for testing:

maintainers = [ ];

Thanks for adding yourself as a module maintainer.

I'm not sure whether this can fit into mkTarget usage. This module defines config.lib.stylix.i3status-rust.bar.

Documentation doesn't seem to refer to this specifically.

And I did not find any such examples. Perhaps this module should remain as is for the time being?

Semantically, this is a target but a "moving target". Perhaps support for those could be added later.

Migrating this pattern to mkTarget has been posponed due to complications:

The infinite recursion here seems to be coming from setting config.lib.stylix, and it may be because setting it is based on an option in config.lib.stylix (colors). I'm not sure what could be done here.

It's a fundamental issue of having the config.lib.stylix.i3.bar, I'll drop i3 from this pr and look into fixing it in a separate pr.

-- #1371 (comment)

This seems to be continued in #1465.

@trueNAHO trueNAHO merged commit 5869510 into nix-community:master Jun 8, 2025
5 checks passed
@mightyiam mightyiam deleted the i3status-rust-opacity branch June 8, 2025 16:01
@mightyiam
Copy link
Copy Markdown
Member Author

Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic: home-manager Home Manager target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants