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.

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

xfce-terminal support #135

Closed
dan-da opened this issue Sep 12, 2023 · 15 comments · Fixed by #585
Closed

xfce-terminal support #135

dan-da opened this issue Sep 12, 2023 · 15 comments · Fixed by #585
Labels
feature New feature request waiting on op Waiting for more information from the original poster
Milestone

Comments

@dan-da
Copy link

dan-da commented Sep 12, 2023

xfce-terminal now supports sixel. Opening this issue as requested on reddit.

references:

@sxyazi
Copy link
Owner

sxyazi commented Sep 12, 2023

Thanks for the issue.

Please do a echo $TERM, $TERM_PROGRAM, $XDG_SESSION_TYPE in xfce-terminal and paste the result here let me know how to adapt it.

@dan-da
Copy link
Author

dan-da commented Sep 12, 2023

$ echo $TERM, $TERM_PROGRAM, $XDG_SESSION_TYPE
xterm-256color, , x11

@dan-da
Copy link
Author

dan-da commented Oct 11, 2023

hmm, with a one-line patch, I was able to get images to display in xfce4-terminal, but it doesn't seem to properly clear the preview area between images. In other words, the new image (or text) just displays over the top of the old one. It seems as if the call to Term::kill_to_end() is ignored. I'm wondering if this is a bug in xfce4-terminal, and or/if there might be another way to clear the preview area, such as drawing a bunch of spaces... I don't really understand yet how ratatui, crossterm, sixel all work to even be able to try that out.

Anyway, here's the little patch I made:

diff --git a/adaptor/src/adaptor.rs b/adaptor/src/adaptor.rs
index 1c361bc..672bf76 100644
--- a/adaptor/src/adaptor.rs
+++ b/adaptor/src/adaptor.rs
@@ -53,6 +53,7 @@ impl Adaptor {
                match term.as_str() {
                        "xterm-kitty" => return Self::Kitty,
                        "foot" => return Self::Sixel,
+                       "xterm-256color" => return Self::Sixel,
                        _ => {}
                }
                match env::var("XDG_SESSION_TYPE").unwrap_or_default().as_str() {

@sxyazi
Copy link
Owner

sxyazi commented Oct 11, 2023

Thanks for your effort, I will install a Linux to test it.

@dan-da
Copy link
Author

dan-da commented Oct 11, 2023

thx, I'm not sure which linux distro(s) might have it already. Probably there are some. You will need xfce4-terminal 1.1.0+ and xfce 4.18+.

I am using ubuntu 22.04 and I built it following these instructions:

https://www.reddit.com/r/xfce/comments/15y1wxh/comment/jxszjgp/?utm_source=share&utm_medium=web2x&context=3

But first I had to upgrade xfce-desktop.
https://www.omgubuntu.co.uk/2022/12/how-to-install-xfce-4-18-on-ubuntu-22-04-22-10

@sxyazi sxyazi added this to the v0.1.6 milestone Oct 13, 2023
@sxyazi
Copy link
Owner

sxyazi commented Oct 15, 2023

I've encountered two issues while trying to adapt Xfce Terminal, which are preventing me from proceeding:

  1. I found that there is no specific identifier for Xfce Terminal other than $TERM being set to xterm-256color. Unlike other terminal emulators, which have unique environment variables like KITTY_WINDOW_ID for Kitty, KONSOLE_VERSION for Konsole, or WEZTERM_EXECUTABLE for WezTerm, seems that xterm-256color is not a valid identifier, as many terminal emulators also use it.

  2. I attempted to build it from source as per the posts in my Arch Linux, and apply the patches for Yazi, but I still can't display images, I'm not sure where I went wrong. My version is 1.1.0.

@dan-da
Copy link
Author

dan-da commented Oct 15, 2023

  1. yeah, I noticed that also. Unsure what to do about it, except perhaps submit a patch or issue to xfce4-terminal.

  2. What version of xfce are you running? If you go to 'Colors' tab of xfce4-terminal settings, do you see a checkbox for 'Enable sixel graphics'?

For me, I first installed xfce 4.18 without rebooting, and then noticed that xfce4-terminal was still not recent enough. So I bulit xfce4-terminal 1.1.0 and ran it, still without any reboot. I found that a) sixel was not working, and there was no settings option to enable it, and b) the about box showed 1.1.0. It was not until I finally rebooted, thus now running xfce 4.18, that xfce4-terminal started displaying sixel, and the "enable sixel graphics" setting appeared in the 'Colors' tab of settings.

@sxyazi
Copy link
Owner

sxyazi commented Oct 18, 2023

I completely reinstalled the latest Manjaro Xfce Edition and compiled by the following steps, but I still can't get it to work:

sudo pacman -S gcc make cmake autoconf automake pkgconf meson gobject-introspection vala xfce4-dev-tools

cd ~/Desktop
git clone https://github.com/GNOME/vte.git
cd vte
meson build -Dsixel=true
ninja -C build install
sudo ldconfig

cd ~/Desktop
git clone https://gitlab.xfce.org/apps/xfce4-terminal.git
cd xfce4-terminal
git checkout xfce4-terminal-1.1.0
./autogen.sh
make -j6
./terminal/xfce4-terminal

My version:

vte 0.75.0
xfce 4.18
xfce-terminal 1.1.0
gtk 3.24.38
kernel 6.1.12-1-MANJARO-ARM

I have done a restart after that, but I still can't find the "Enable sixel graphics" checkbox in "Colors" tab:

image

@dan-da
Copy link
Author

dan-da commented Oct 19, 2023

there was one more step in the linked post. might be what you're missing:

cd ~/build
git clone https://github.com/koppa/matplotlib-sixel.git
cd ./matplotlib-sixel
#fix the ä in the setup.py script...
sudo python3 setup.py install

Also, I see this comment was added 4 days ago. Maybe not relevant:

cd ~/build
git clone https://github.com/GNOME/vte.git
cd vte



and then

git checkout 3745502

(last commit before gtk4, this requires a gtk4 version not available on ubuntu 22.04)

@sxyazi
Copy link
Owner

sxyazi commented Oct 19, 2023

there was one more step in the linked post. might be what you're missing:

Also, I see this comment was added 4 days ago. Maybe not relevant:

I've tried both of these, but they don't work. I also attempted to uninstall the XFCE terminal that comes with the system, but it didn't work either.

I recorded a video to demonstrate this:

3.out.mp4

@dan-da
Copy link
Author

dan-da commented Oct 19, 2023

hmm, I watched your video and it looks like pretty much what I did. So I don't really have any further suggestions.

I opened a gitlab issue for xfce4-terminal project. Maybe someone there can assist.

@sxyazi
Copy link
Owner

sxyazi commented Oct 23, 2023

but it doesn't seem to properly clear the preview area between images. In other words, the new image (or text) just displays over the top of the old one

I think #309 should address this issue.

All we need to do next is wait for xfce-terminal to add an environment variable, and then add corresponding checks in Yazi! 🚀

@dan-da
Copy link
Author

dan-da commented Oct 23, 2023

I confirm that #309 fixes it for me. thanks!

@sxyazi sxyazi modified the milestones: v0.2.0, backlog Jan 11, 2024
@sxyazi
Copy link
Owner

sxyazi commented Jan 26, 2024

Hi, I created a PR and tried to detect Sixel support through CSI, let me know if it works for you, #585

@sxyazi sxyazi added waiting on op Waiting for more information from the original poster feature New feature request labels Jan 26, 2024
Copy link

github-actions bot commented Mar 1, 2024

I'm going to lock this issue because it has been closed for 30 days. ⏳ This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New feature request waiting on op Waiting for more information from the original poster
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants