Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add Sixel Support #2157

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add Sixel Support #2157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions guake/data/org.guake.gschema.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,11 @@
<summary>Audible bell</summary>
<description>If true, the system alert sound will be played on a bell character.</description>
</key>
<key name="use-sixel" type="b">
<default>false</default>
<summary>Use Sixel</summary>
<description>If true, terminals will render sixel escape sequences.</description>
</key>
<key name="window-width" type="i">
<default>100</default>
<summary>Window width.</summary>
Expand Down
24 changes: 23 additions & 1 deletion guake/data/prefs.glade
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@
<property name="top-padding">12</property>
<property name="left-padding">12</property>
<child>
<!-- n-columns=3 n-rows=5 -->
<!-- n-columns=3 n-rows=6 -->
<object class="GtkGrid">
<property name="visible">True</property>
<property name="can-focus">False</property>
Expand Down Expand Up @@ -533,6 +533,28 @@
<property name="top-attach">4</property>
</packing>
</child>
<child>
<object class="GtkCheckButton" id="use_sixel">
<property name="label" translatable="yes">Render sixel escape sequences (images)</property>
<property name="visible">True</property>
<property name="can-focus">True</property>
<property name="receives-default">False</property>
<property name="halign">start</property>
<property name="use-underline">True</property>
<property name="draw-indicator">True</property>
<signal name="toggled" handler="on_use_sixel_toggled" swapped="no"/>
</object>
<packing>
<property name="left-attach">0</property>
<property name="top-attach">5</property>
</packing>
</child>
<child>
<placeholder/>
</child>
<child>
<placeholder/>
</child>
<child>
<placeholder/>
</child>
Expand Down
8 changes: 8 additions & 0 deletions guake/prefs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,10 @@ def on_use_audible_bell_toggled(self, chk):
"""Changes the value of use_audible_bell in dconf"""
self.settings.general.set_boolean("use-audible-bell", chk.get_active())

def on_use_sixel_toggled(self, chk):
"""Changes the value of enable_sixel in dconf"""
self.settings.general.set_boolean("use-sixel", chk.get_active())

# scrolling tab

def on_use_scrollbar_toggled(self, chk):
Expand Down Expand Up @@ -1182,6 +1186,10 @@ def load_configs(self):
value = self.settings.general.get_boolean("use-audible-bell")
self.get_widget("use_audible_bell").set_active(value)

# use sixel
value = self.settings.general.get_boolean("use-sixel")
self.get_widget("use_sixel").set_active(value)

self._load_screen_settings()

value = self.settings.general.get_boolean("quick-open-enable")
Expand Down
9 changes: 9 additions & 0 deletions guake/terminal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,15 @@ def configure_terminal(self):
if hasattr(self, "set_alternate_screen_scroll"):
self.set_alternate_screen_scroll(True)

enable_sixel = client.get_boolean("use-sixel")
Copy link
Collaborator

@Davidy22 Davidy22 Feb 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Does this need to be a preferences option? Seems like the kind of thing that anyone who has a terminal that can support it would want. If we do keep this as an option in preferences, we also need to add some switches on setting toggle, because as-written currently if a user clicks the box to enable they still won't see sixels until they restart guake.

I'm leaning towards always on and throw a low log message if the toggle fails just because I feel like everyone who knows these exist would be fine having them. Preferences windows become scary when there's too many things in them

Copy link
Author

Choose a reason for hiding this comment

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

I figured it's probably a good idea to have this optional at least as long as sixel support is still considered experimental in vte.

Copy link
Collaborator

@Davidy22 Davidy22 Feb 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

While it is experimental right now, it's also quite unavailable, I think the only way currently for people to have a version of general VTE that supports sixels installed is to compile it yourself, which is tantamount to opting in, and when it lands in an official VTE release pushed to general users I would think it is no longer experimental.

Copy link
Author

Choose a reason for hiding this comment

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

That makes sense. I'll remove the option to enable them and just turn them on when available.

Copy link
Author

Choose a reason for hiding this comment

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

I'd probably still keep the setting in the gschema, if advanced users want to still opt-out somehow / for some reason. But I'll default it to enabled, would that be okay?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Alright, sure. And a memo for another day to change some things when sixels come to VTE proper.

if enable_sixel:
# TODO: Once Sixel support is stable and there's a release with it built-in by default,
# we can use a version check like done above for other features.
try:
self.set_enable_sixel(True)
except: # pylint: disable=bare-except
log.warn("set_enable_sixel is not supported by your version of VTE")

self.set_can_default(True)
self.set_can_focus(True)

Expand Down
9 changes: 9 additions & 0 deletions releasenotes/notes/sixel_support-467783a27379bbe2.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
release_summary: >
Add experimental support for displaying Sixels. This can be enabled with a new setting.

features:
- add sixel support #1806

known_issues:
- sixel support requires VTE to be compiled with `-Dsixel=true`, since support for it is still experimental.