alphapapa
Added Opacity parameter parameter to enable "rainbows" theme
Hey, I wanted a background theme for your package, but tweaking the available lightens and desaturations parameters, it was just way too obtrusive, so I hacked around on it a bit to add "opacity", basically prism-blend with the default background attribute. I found it pretty useful, and I was really pleased with the effect for my theme so thought I'd share. Here are my parameters:
(use-package prism
:straight (prism :type git
:host github
:branch "rainbows"
:repo "Overdr0ne/prism.el")
:config
(setf prism-num-faces 30
prism-color-attribute :background
prism-colors (list "maroon" "violet" "blue violet" "blue" "light sea green" "green" "yellow green" "yellow" "orange" "red")
prism-desaturations (list 0 0 0)
prism-lightens (list 0 0 0)
prism-opacities (list 10 30 90)
prism-comments-fn (lambda (color) color)
prism-strings-fn (lambda (color) color)
prism-parens-fn (lambda (color) color))
(prism-save-colors))
I've also included a demo gif in the images folder: https://github.com/Overdr0ne/prism.el/blob/rainbows/images/rainbows-demo.gif You'll notice I switch between my light and my dark theme there, and because of the opacity parameter, it works for both.
Hey, thanks. This is an interesting idea, and I'm probably okay with adding it. However, this PR has a bunch of unrelated changes. If you want to propose them as well, you should do them in separate PRs--although they seem unnecessary as well, so I'm not sure why you made them.
Cool, yeah, it still has some of my hackery in there, I'll clean it up here in a bit and let you know.
Okay, I think I've about finished with the requested changes, lmk if you see anything else.
You'll also notice a new commit resolving a bug that was giving me some trouble, particularly when switching themes. I uncovered it when I went back to test things with the default parameters. I suspect it is already affecting other users if their theme does not specify one of the faces that prism theming uses- for me, it was the font-lock-comment-face set with prism-comment-fn. Using that default would put me into a loop when prism-customize-set tried and failed to save things, effectively preventing me from changing the default through customize without restarting emacs. Take a look and see what you think.
Let me begin by asking this, before I forget: Have you signed the FSF copyright assignment? Since I now have a package in GNU ELPA, I'm thinking about contributing some more of mine to it, and this one seems like an obvious candidate. This would be a non-trivial change according to FSF guidelines, so it would require CA if it were in ELPA. (I apologize for not raising this issue sooner, but this is a recent development for me, and I'm still considering how it applies to my packages and to which ones.)
You'll also notice a new commit resolving a bug that was giving me some trouble, particularly when switching themes. I uncovered it when I went back to test things with the default parameters. I suspect it is already affecting other users if their theme does not specify one of the faces that prism theming uses- for me, it was the font-lock-comment-face set with prism-comment-fn. Using that default would put me into a loop when prism-customize-set tried and failed to save things, effectively preventing me from changing the default through customize without restarting emacs. Take a look and see what you think.
If you've found a bug, please report and patch it separately from this PR.
I'll make some more comments as reviews. Thanks.
By the way, regarding the GIF demo, I think there are a couple of issues:
- It seems a bit much to digest. It moves very quickly when you scroll, and the image includes much content that's unrelated to the demo. A non-animated screenshot should be enough, if the code sample is carefully chosen.
- It doesn't actually look very attractive to a potential user because of the way background colors are wrapped onto new lines and applied to whitespace at the beginning of subsequent lines. I don't think I could use this feature myself, because it's too much visual noise to be useful. That's why I hadn't already added a screenshot showing use of the background face attribute.
It might be worth trying to support this use case more intentionally by changing the code to not apply the background color to indentation, as long as doing so wouldn't reduce performance in other cases.
Let me begin by asking this, before I forget: Have you signed the FSF copyright assignment? Since I now have a package in GNU ELPA, I'm thinking about contributing some more of mine to it, and this one seems like an obvious candidate. This would be a non-trivial change according to FSF guidelines, so it would require CA if it were in ELPA. (I apologize for not raising this issue sooner, but this is a recent development for me, and I'm still considering how it applies to my packages and to which ones.)
I have not yet signed it but I'll look into it and get back to you...
By the way, regarding the GIF demo, I think there are a couple of issues:
- It seems a bit much to digest. It moves very quickly when you scroll, and the image includes much content that's unrelated to the demo. A non-animated screenshot should be enough, if the code sample is carefully chosen.
The reason I made it animated was to show that rainbows works, without adjustment, when I switch from a very light to a very dark theme. That is because of the opacity parameter. It is a bit fast though, I probably could get away with two static images.
- It doesn't actually look very attractive to a potential user because of the way background colors are wrapped onto new lines and applied to whitespace at the beginning of subsequent lines. I don't think I could use this feature myself, because it's too much visual noise to be useful. That's why I hadn't already added a screenshot showing use of the background face attribute.
It might be worth trying to support this use case more intentionally by changing the code to not apply the background color to indentation, as long as doing so wouldn't reduce performance in other cases.
I definitely agree that the whitespace highlighting is not ideal, but I suspect that would be beyond the scope of this PR. I may look into it, but later. As for noise, the colors in that gif don't really accurately depict what it actually looks like on my screen. To me, the colors are very subtle for levels 0-9, where I really only notice them if I look at them, which was my intention. But I may need to color adjust for the screenshots.
I've added some more PRs that this PR will depend on. Once we've sorted through those, I'll rebase this and test everything again.
For the copyright assignment, do you have access to the paperwork for that? I would be happy to sign it.
@Overdr0ne Sorry that I missed your last comment. Feel free to ping me in the future as necessary.
You can request the copyright assignment form by sending an email to [email protected] asking for it. Usually it can all be handled via email and doesn't take too long, depending on how busy the FSF secretary is.
If you can get that, we can talk about merging the other PRs, and then maybe this one. :) Thanks.
Hey, no problem, I went ahead and requested the paperwork. I've never done that before-- will I need to assign copyright for every GNU package I may contribute to separately? I guess I'll see how exactly that process goes from the secretary...
An assignment covers any submission to Emacs or packages that are in GNU ELPA (which are considered "part of Emacs").