kovidgoyal / kitty

Cross-platform, fast, feature-rich, GPU based terminal
https://sw.kovidgoyal.net/kitty/
GNU General Public License v3.0
24.16k stars 972 forks source link

Image preview does not work on lf #6010

Closed rockyzhang24 closed 1 year ago

rockyzhang24 commented 1 year ago

After upgrading to 0.27.0, the image preview in lf file manager doesn't work. I will link the issue (https://github.com/gokcehan/lf/issues/1099) here as a reference and please check it for details.

The command used for image preview was kitty +icat --silent --transfer-mode file --place "${w}x${h}@${x}x${y}" "$file" and it worked well before 0.27.0. After 0.27.0, I adjusted kitty +icat to kitty +kitten icat and it should work as before, but not. I tried kitty +kitten icat --detect-support and it output memory, but it was file before 0.27.0. I am not sure whether it is the reason causing this issue.

Actually, I am not sure whether this is a bug introduced by 0.27.0 and can be fixed by kitty, or not a bug and lf should handle it on its side.

Thank you very much.

kovidgoyal commented 1 year ago

There is no bug in kitty here. lf needs to be updated to either support memory based transfers, or pass the --transfer-mode argument to the kitten to force it to use file based transferring.

rockyzhang24 commented 1 year ago

Thank you for your response. I use --transfer-mode file all the time in the script for image preview in lf, but the image isn't displayed in the preview window after 0.27.0. Anyway, hopefully lf could fix this.

page-down commented 1 year ago

I can reproduce this issue.

When using icat kitten that comes with kitty 0.26.5, the image is displayed normally, but when changing to kitten from master, the image is not displayed.

~/.config/lf/lfrc

set cleaner ~/.config/lf/lf_kitty_clean
set previewer ~/.config/lf/lf_kitty_preview

~/.config/lf/lf_kitty_clean

#!/bin/sh
exec kitten icat --clear

~/.config/lf/lf_kitty_preview

#!/usr/bin/env bash
set -eu
[[ "$(file -Lb --mime-type "$1")" =~ ^image ]] || exit 1   
exec /path/to/kitty-0.26.5/bin/kitty +kitten icat --silent --transfer-mode file --place "${2}x${3}@${4}x${5}" "$1"
chmod +x ~/.config/lf/lf_kitty_{clean,preview}

Running in kitty from master, lf version r28.

So how should lf users change the script to use --transfer-mode file to display images?

kovidgoyal commented 1 year ago

The linked lf issue claimed the problem is lf does detection of available transfer modes and is thrown off by the use of the memory transfer mode. Which is why I advised using the file mode. If that's not the issue then someone needs to investigate what the actual issue is. Capture stderr when running the icat command inside lf by wrapping it in a script. That will tell you if its failing and why.

page-down commented 1 year ago

Capture stderr when running the icat command inside lf by wrapping it in a script.

I recorded both command and stderr, not sure where the two images came from.

kitten icat --transfer-mode file --place 41x22@39x1 /path/to/png

Error: The --place option can only be used with a single image, not 2
kovidgoyal commented 1 year ago

kitten icat --transfer-mode file --place 41x22@39x1 /path/to/png

works for me. If your are getting the above error that means that is not the actual command line being executed by lf. It is likely appending something to that command line,

rockyzhang24 commented 1 year ago

works for me

It works when we run it directly in the terminal. However, if we put it in the script as the previewer of lf (as what @page-down described here https://github.com/kovidgoyal/kitty/issues/6010#issuecomment-1429163027), the error occurs.

Thank you.

kovidgoyal commented 1 year ago

works for me

It works when we run it directly in the terminal. However, if we put it in the script as the previewer of lf, the error occurs.

Then lf is obviously not actually executing that command line. Use strace -f on lf see what actual command line it is executing.

page-down commented 1 year ago

It is likely appending something ...

I tried to use sh in the script to execute it, which would not be possible to add any more arguments.

sh -c 'kitten icat --transfer-mode file --place 41x22@39x1 /path/to/png'

It looks like /dev/stdin is added to items as an image, so the count is 2. If I use the option --stdin no, then the escape sequence text is printed in the specified place instead of displaying the image.

kovidgoyal commented 1 year ago

Ah, that would mean lf is executing it with STDIN being a pipe or a file rather than the tty.

kitten icat --transfer-mode file --stdin no --place 41x22@39x1 file.png < /dev/null

works fine for me. So change the kitten invocation to that.

page-down commented 1 year ago

So change the kitten invocation to that.

I changed it as above in the preview script. As I said before, --stdin no will make the text be printed in the specified place instead of the image. Since I'm not familiar with lf or icat, I don't know what the problem is.

I also found another problem. After executing kitten icat --place ... at a shell prompt, the cursor position is not restored.

I tested it under bash and the next prompt was printed under the image (second line from the top left of the image). If executed under fish, a fish-specific "carriage return" Unicode symbol is printed in the top left corner of the image.

With kitty 0.26.5, the cursor is back in place, and the next prompt in bash and fish is normal, without the fish-specific "carriage return" being printed in the image's specified position.

@kovidgoyal This is different from the previous icat behavior. Is this what is expected?

kovidgoyal commented 1 year ago

What text is being printed?

And if you are using --place the behavior has been changed to keep the cursor where it is, this is deliberate, as this prevents the screen from scrolling when the image is the same size or larger than the screen. This is documented in the --place options docs. --place is not meant to be used at shell prompts, it never was.

kovidgoyal commented 1 year ago

If the text being printed is the escape codes for image display, that would imply lf is taking the STDOUT of the script escaping escape codes in it and printing it. Adding > /dev/tty should do the trick

kitten icat --transfer-mode file --stdin no --place 41x22@39x1 file.png < /dev/null > /dev/tty
rockyzhang24 commented 1 year ago

Wow, awesome. It works perfectly. Thank you for your patience and instruction ♥️

kovidgoyal commented 1 year ago

You're welcome. Note that you can probably remove --transfer-mode file from this command line for even better performance then it will use shared memory automatically when available.

rockyzhang24 commented 1 year ago

you can probably remove --transfer-mode file from this command line

I tried it but it crashed. I attached a demo below.

https://user-images.githubusercontent.com/11582667/218681789-daa45fe2-b3c7-4c8c-aeee-3cc66ad93eb6.mov

kovidgoyal commented 1 year ago

OK I'm afraid I dont have the time/interest to debug lf to see what is going on, just use --transfer-mode file.

rockyzhang24 commented 1 year ago

Sure. Thanks a lot for all the help.

page-down commented 1 year ago

... debug lf to see what is going on ...

For this question, I don't think debugging lf is needed. If kitten is not attached to TTY, how does it receive detect replies from kitty the terminal?

@rockyzhang24

I tried it but it crashed. ...

After receiving the reply from kitty, lf did not discard the unrecognized escape sequence, but performed some actions, such as opening the image file using a text pager.

I think all questions about icat have been answered here, and others related to lf will be discussed in its project‘s issue.

kovidgoyal commented 1 year ago

On Tue, Feb 14, 2023 at 04:24:12AM -0800, page-down wrote:

... debug lf to see what is going on ...

For this question, I don't think debugging lf is needed. If kitten is not attached to TTY, how does it receive detect replies from kitty the terminal?

It is attached to tty, otherwise > /dev/tty would not work. Also, the kitten will given an error when it cannot open /dev/tty and it needs to query the tty. And in the OPs video you can see that the first two images displayed the problem happens when either opening an image or going to the third image, I cant be sure which.