-
Notifications
You must be signed in to change notification settings - Fork 578
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
Add background image to guake #1604
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.
This looks amazing, but I do not see fallback mechanism for user with old version of VTE (or it should work with any version, isn’t it?) or notification that this part of the gschema and preference might not work on the user’s system
It should be used in any modern VTE version (it doesn't invoke any VTE methods), and anyone should able to use it without problem. |
Oh ok you don’t use any vte function. I guess this may take some cpu to run, can check if when disabled it really does not have any impact on perf |
3040337
to
f3e4d62
Compare
It now should cached when loading the same file (e.g. when restoring tabs). |
f3e4d62
to
a7799e8
Compare
guake/data/org.guake.gschema.xml
Outdated
@@ -166,6 +166,16 @@ | |||
<summary>Window vertical alignment.</summary> | |||
<description>Place Guake at: 0: top, 1: bottom</description> | |||
</key> | |||
<key name="background-image-file" type="s"> | |||
<default>''</default> |
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.
Indents are all off here
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.
Tried it out and was sort of hoping for image transparency, but this is still nice and pretty without that. Would be nice if we used a file picker that showed thumbnails too. Both of those can get added in another commit though.
In addition to polishing off that whitespace objection from @la5942, also going to need to grab the recent CI changes and make sure this passes the build and then we can merge. |
Went and committed the little whitespace edit myself, just some merges and then a passthrough of black left and this should be good to go. |
The implementation is based on tilix #576. For detail explanation, please look into BackgroundImageManager.draw, which have document the comment on it.
Adding this myself since I'm able to
eff77ba
to
2b51a07
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.
Went and did the rebase myself. Still works, passes checks, was already approved before so merging.
Impl #1603.
Impl detail please refer to
guake/utils.py:BackgroundImageManager.draw