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

Support for WezTerm #41

Open
notanimposter opened this issue Feb 27, 2023 · 7 comments
Open

Support for WezTerm #41

notanimposter opened this issue Feb 27, 2023 · 7 comments

Comments

@notanimposter
Copy link

It would be really cool if this program supported the WezTerm terminal emulator.

WezTerm supports the Kitty image protocol out of the box (and the Kitty keyboard protocol with a config option set), so this shouldn't be difficult. In fact, I'm surprised it doesn't work already, but in my testing it seems that keyboard input is not working at the moment.

@DanCardin
Copy link

DanCardin commented Mar 22, 2023

i dont know what \033\\ is supposed to mean/be but it was caught in a loop expecting it. Changing to this, "fixed" for me, although i dont know whether this could have other effects.

diff --git a/termpdf.py b/termpdf.py
index 96ee9e0..0860d66 100755
--- a/termpdf.py
+++ b/termpdf.py
@@ -1088,7 +1088,7 @@ def write_gr_cmd_with_response(cmd, payload=None):
     # rewrite using swallow keys to be nonblocking
     write_gr_cmd(cmd, payload)
     resp = b''
-    while resp[-2:] != b'\033\\':
+    while resp[-2:] != b'':
         resp += sys.stdin.buffer.read(1)
     if b'OK' in resp:
         return True

EDIT: also, since having posted this, i'm now encountering issues with the above change, although it's certainly still the problem, which i'm not surprised about. but that's at least why it's not reacting to user input or file changes...


I also made this change:

@@ -1522,8 +1522,12 @@ def view(file_change,doc):
         
         scr.stdscr.nodelay(True)
         key = scr.stdscr.getch()
-        while key == -1 and not file_change.is_set():
-            key = scr.stdscr.getch()
+        key = scr.stdscr.getch()
+        if key == -1 and not file_change.is_set():
+            while key == -1 and not file_change.is_set():
+                sleep(1)
+                key = scr.stdscr.getch()
+
         scr.stdscr.nodelay(False)
 
         if file_change.is_set():

to prevent it from looping looking for input. but again, im not familiar with curses and there might be a better way.

I'd almost submit a proper PR but i dont know the quality of the above changes, and the owner hasn't responded to other PRs recently anyway.

@jyue86
Copy link

jyue86 commented Apr 25, 2023

Which commit are you checked out at? I've tried using the newest commit and commit @cbb19e0 with the suggested modified changes, but then, the preview stops showing. I'm wondering if there was a change that was introduced recently that causes the code changes to not work, although it's not likely looking at the commit log.

@DanCardin
Copy link

My current working code (against master) is produces the following diff:

diff --git a/termpdf.py b/termpdf.py
index 96ee9e0..d33556b 100755
--- a/termpdf.py
+++ b/termpdf.py
@@ -1088,7 +1088,7 @@ def write_gr_cmd_with_response(cmd, payload=None):
     # rewrite using swallow keys to be nonblocking
     write_gr_cmd(cmd, payload)
     resp = b''
-    while resp[-2:] != b'\033\\':
+    while resp[-2:] != b'':
         resp += sys.stdin.buffer.read(1)
     if b'OK' in resp:
         return True
@@ -1495,10 +1495,6 @@ def view(file_change,doc):
     scr.get_size()
     scr.init_curses()
 
-    if not detect_support():
-        raise SystemExit(
-            'Terminal does not support kitty graphics protocol'
-            )
     scr.swallow_keys()
 
     bar = status_bar()
@@ -1522,7 +1518,9 @@ def view(file_change,doc):
         
         scr.stdscr.nodelay(True)
         key = scr.stdscr.getch()
+        key = scr.stdscr.getch()
         while key == -1 and not file_change.is_set():
+            sleep(1)
             key = scr.stdscr.getch()
         scr.stdscr.nodelay(False)

although i haven't touched it in a while, and can't say what exactly is necessary or not anymore 😬.

With this, on macos + a month-or-so old wezterm nightly, it appears to continuously update without issue.

@jyue86
Copy link

jyue86 commented Apr 27, 2023

This works for me! I am observing a warning that says that it failed to load the page, but the functionality does not seem to be impacted. I will see if I can find a way to resolve this warning over the weekend.

I think the sleep(1) can be removed. I noticed moving between pages was slow, so after removing that line, moving between pages became instantaneous.

@DanCardin
Copy link

Yea, like I said, this just happens to be the state of my clone at the moment. I believe I put that there though, to keep it from using 100% cpu usage while it loops in between keypresses/file changes. there's probably a better curses-specific thing to do there, but i dont know it.

@sschober
Copy link

Hi!

I found this issue yesterday, as I was stumbling into it trying termpdf.py master with wezterm 20231017-091526-fec90ae0.

My interpretation of the problem is the following:

According to the kitty graphics protocol documentation, the terminal emulator is supposed to answer a "display graphics command" with a byte sequence of the form <ESC>_Gi=<id>;OK<ESC>\.

And the termpdf.py code tries to read an answer:

def write_gr_cmd_with_response(cmd, payload=None):
    # rewrite using swallow keys to be nonblocking
    write_gr_cmd(cmd, payload)
    resp = b''
    while resp[-2:] != b'\033\\':
        resp += sys.stdin.buffer.read(1)
    if b'OK' in resp:
        return True
    else:
        return False

but never returns from read(1). It just hangs there.

So my suspicion is, that maybe wezterm does not answer with this byte sequence. I'll dig deeper into that.

For me, the current work around is the following diff:

@@ -1087,6 +1095,7 @@ def write_gr_cmd(cmd, payload=None):
 def write_gr_cmd_with_response(cmd, payload=None):
     # rewrite using swallow keys to be nonblocking
     write_gr_cmd(cmd, payload)
+    return True
     resp = b''
     while resp[-2:] != b'\033\\':
         resp += sys.stdin.buffer.read(1)

But that is not optimal, as it removes error checking, as the terminal emulator might signal, that it was unable to display the image.

@sschober
Copy link

After reading the kitty graphics protocol more closely, I am no longer sure, that a repsonse is even mandatory. So, I've change my "fix" to be:

@@ -681,7 +681,7 @@ class Document(fitz.Document):
             self.clear_page(self.prevpage)
             # display the image
             cmd = {'a': 'p', 'i': p + 1, 'z': -1}
-            success = write_gr_cmd_with_response(cmd)
+            success = write_gr_cmd(cmd)
             if not success:
                 self.page_states[p].stale = True
                 bar.message = 'failed to load page ' + str(p+1)

I've created a pull request (#44) for this.

I did not do a lot of testing yet, so please, let me know, if you see any regressions with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants