-
-
Notifications
You must be signed in to change notification settings - Fork 166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
zathura: add transparency to highlight colors #394
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not tested this yet, but I have two suggestions:
-
Reduce the let-in scope by adding it to
programs.zathura.options = let ... in {
. -
Zathura recently-ish deprecated their
highlight-transparency
option, which by default was set to something like0.3
Since this is an upstream breaking change, Stylix should not enforce the previous deprecated behaviour by default. Add a Stylix option for declaring
zathura
'shighlight-transparency
option. Ideally, theexample
value should be set to the previous default value.Could you maybe find the
zathura
commit introducing this breaking change and use the exact value? It would also be good for reference, to link the commit in this PR and thedescription
of the new option.
Thanks for the feedback!
Ok!
Here's the relevant commit in zathura. I now realise I was wrong with the EDIT: I now also realise that there's a 3rd option to which they added transparency, |
Yes, this looks like something which we should define. |
Upon reconsideration, Stylix should not enforce unnecessary options:
People expecting zathura's non-transparent default behaviour would suffer undesired Stylix behaviour, as it tries to mimic zathura's deprecated behaviour by default. Maybe Stylix should not explicitly support zathura transparency, and users should explicitly declare it in their configuration, if desired? |
I'm not sure this is what the situation is. I think zathura's default behaviour (both before and after the deprecation of I think that it would make sense to change stylix so that the behaviour is the same as before the zathura change. I do not think that having transparency disabled is something that anyone wants (as evidenced by the fact that it's zathura's default behaviour) -- it makes text unreadable and is frankly quite annoying. Of course, there is something to be said for stylix to allow users to configure the level of transparency, but I'm not sure that is a necessary option. So the end decision is with you guys of course :). |
Tested and works great with vimtex 👍 Should be fine to merge this now since 24.05 is stable and comes with the new zathura |
fd59863
to
9d00661
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, making it configurable is not necessary for now since it sounds like this is just replicating the previous default value.
The test here seems to have timed out, is that the only blocker? |
Yeah, everything else is fine. I'll restart the test. Related- #403 |
This adds transparency back to the zathura module as discussed. I'm new to nix, so let me know if there's something to improve :).
Closes: #392