dsanson / termpdf.py

A graphical pdf and epub reader that works inside the kitty terminal
MIT License
506 stars 32 forks source link

Support for WezTerm #41

Open notanimposter opened 1 year ago

notanimposter commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 11 months ago

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 commented 11 months ago

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.