termpdf.py icon indicating copy to clipboard operation
termpdf.py copied to clipboard

Support for WezTerm

Open notanimposter opened this issue 1 year ago • 7 comments

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.

notanimposter avatar Feb 27 '23 21:02 notanimposter

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.

DanCardin avatar Mar 22 '23 14:03 DanCardin

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.

jyue86 avatar Apr 25 '23 08:04 jyue86

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.

DanCardin avatar Apr 26 '23 16:04 DanCardin

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.

jyue86 avatar Apr 27 '23 04:04 jyue86

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.

DanCardin avatar Apr 27 '23 16:04 DanCardin

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 avatar Oct 28 '23 04:10 sschober

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.

sschober avatar Oct 28 '23 10:10 sschober