gokcehan / lf

Terminal file manager
MIT License
7.69k stars 326 forks source link

kitty-0.27.0 preview not working due to change in icat's transfer mode #1099

Open ignamartinoli opened 1 year ago

ignamartinoli commented 1 year ago

Hi, I was having a problem with my previewer script, which is rather complex.

So I just commented everything and tried the basic example script from the documentation:

#!/bin/sh

file=$1
w=$2
h=$3
x=$4
y=$5

if [[ "$( file -Lb --mime-type "$file")" =~ ^image ]]; then
    kitty +icat --silent --transfer-mode file --place "${w}x${h}@${x}x${y}" "$file" && echo "$file" >> lemmesee
    exit 1
fi

As you can see, I added the && echo 'success' >> lemmesee at the bottom of the command.

The file never gets created in the first place when I hover on some image. The weird part is that, if I manually run that command it works flawlessly.

This is the relevant configuration file:

set shell sh
set shellopts '-eu'
set ifs "\n"
set icons
set mouse

set scrolloff 10
set drawbox

set ignorecase true

set previewer ~/.config/lf/preview.sh
set cleaner ~/.config/lf/clean.sh
Ultra-Code commented 1 year ago

I am having the exact same issue. I am able to view images in my kitty terminal and in the shell created by clicking w, But Lf isn't able to display the generated image even though I can view it directly in my terminal with kitty. I verified that the thumbnail was generated like expected but Lf is unable to display the image. I have a gut feeling that it might have something to do with the new improvements in kitty.

@ignamartinoli NOTE: due to a recent change in kitty 0.27.0, the '+kitten' subcommand is needed to run all kitty kittens .ie $ kitty +kitten icat but this still doesn't work.

- A new statically compiled, standalone executable, kitten (written in Go) that 
can be used on all UNIX-like servers for remote control (kitten @), 
viewing images (kitten icat), manipulating the clipboard (kitten clipboard), etc.

- icat kitten: Speed up by using POSIX shared memory when possible to transfer 
image data to the terminal. Also support common image formats 
GIF/PNG/JPEG/WEBP/TIFF/BMP out of the box without needing ImageMagick.
ignamartinoli commented 1 year ago

@Ultra-Code I can confirm, this is version specific.

I regressed to version 0.26.5 and it works again

Ultra-Code commented 1 year ago

@ignamartinoli can you verify for me the output of kitty +kitten icat --detect-support on your system. On my system the result of the command is memory, :thinking: but I suspect that Lf expects a file transfer mode instead of memory and that might be what is preventing Lf from working like expected with the latest kitty.

ignamartinoli commented 1 year ago

You're right!

Not working:

sicro@sicro ~ $ kitty -v
kitty 0.27.0 created by Kovid Goyal

sicro@sicro ~ $ kitty +kitten icat --detect-support
memory

Working:

sicro@sicro ~ $ kitty -v
kitty 0.26.5 created by Kovid Goyal

sicro@sicro ~ $ kitty +kitten icat --detect-support
file
ignamartinoli commented 1 year ago

What is weird is that in the commands about I explicitly typed --transfer-mode file. I don't know if maybe that becomes overwritten

Ultra-Code commented 1 year ago

@ignamartinoli It seems memory is used almost exclusively now but I would have expected the --transfer-mode option to be able to override the default but It seems like that isn't happening(but am unable to verify that because stream is respected). Lf would have to be modified to be able to work with both memory and file transfer modes.

ignamartinoli commented 1 year ago

Well, at least for now reverting Kitty version is a temporary solution. I'm not sure I would be able to succeed on modifying lf codebase to fix this

Ultra-Code commented 1 year ago

@ignamartinoli I currently don't have enough time on my hand to fix this but I would dig more into this issue when I get some free time and its still not fixed. But for the time being, I suggest you modify the title to better reflect the issue at hand. Eg. Kitty preview not working in 0.27.0 due to recent change in icat's transfer mode

ignamartinoli commented 1 year ago

You're right!

rockyzhang24 commented 1 year ago

Same here. Is this issue caused by lf, and could it be fixed on this side @gokcehan ? Thank you.

rockyzhang24 commented 1 year ago

The author of kitty pointed out a trick to fix this. Please change the command to kitten icat --transfer-mode file --stdin no --place "${w}x${h}@${x}x${y}" "$file" < /dev/null > /dev/tty.

For details, see https://github.com/kovidgoyal/kitty/issues/6010 please.

page-down commented 1 year ago

kitten icat --stdin no ... < /dev/null > /dev/tty

This may also need to be applied to kitten icat --clear I'm guessing.

When I switch between two png image files (up and down arrow keys), the following message appears.

... may be a binary file.  See it anyway?

Then it will lose response and the keys will not work, if I try ctrl-c to terminate, the key codes will be printed at the bottom line.

The kitten icat is currently implemented in Go, and it is probably best to implement the kitty graphics protocol directly, without calling external processes.

Gobidev commented 1 year ago

When I switch between two png image files (up and down arrow keys), the following message appears. ... may be a binary file. See it anyway? Then it will lose response and the keys will not work, if I try ctrl-c to terminate, the key codes will be printed at the bottom line.

I had the same issue when I tried running it without --transfer-mode file, however with it, all works as expected. This is what my scripts currently look like: lf_kitty_clean:

#!/usr/bin/env bash

kitty +kitten icat --clear --stdin no --silent --transfer-mode file < /dev/null > /dev/tty

lf_kitty_preview:

#!/usr/bin/env bash
file=$1
w=$2
h=$3
x=$4
y=$5

if [[ "$( file -Lb --mime-type "$file")" =~ ^image ]]; then
    kitty +kitten icat --silent --stdin no --transfer-mode file --place "${w}x${h}@${x}x${y}" "$file" < /dev/null > /dev/tty
    exit 1
fi
page-down commented 1 year ago

@Gobidev Thanks for sharing the full script.

I modified it to use kitten icat --transfer-mode memory ... and everything works fine. kitten is only suitable for users who know they have the 0.27+ version of kitty installed. This will eliminate the need to start Python interpreter and call kitten (implemented in Go) again.

Those messages appear because:

To make the default --transfer-mode detect work, escape sequences from kitty need to be handled properly.. And then kitten will be possible to automatically change to stream mode when used via ssh, otherwise the images cannot be displayed.

Ultra-Code commented 1 year ago

The new kitty preview and clean command work perfectly but this sometimes crushs the lf instance or the shell when entering/opening a sub shell with the w default map.

To reproduce

open lf in a new terminal window highlight an image for previewer to kick in press the w key

This produces an error zsh: suspended (tty output) lf

when you try to reopen lf imediately afterwards in the shell you are drop in you get the error zsh: error on TTY read: Error from entering/leaving Captura de pantalla de 2023-02-16_23:28:15 Captura de pantalla de 2023-02-16_23:34:42

The link to my lf configuration files

kazmath commented 1 year ago

I know this is from 3 months ago, but I'm having the same problem. The scripts posted here as a workaround do not work. My kitty version is 0.28.1, not 0.27.0 tho, so I wonder if it's the same issue. If not, I'll file another issue, but this one is still open so I'm trying here first.

Only a little black square in the top left corner of the preview shows up when I hover over a file, the same thing also shows up if I try to execute the script with the proper parameters, but it works normally if I execute only the command from the terminal.

The script is the same except for some commented lines with the previous command and an else statement to use pistol as a fallback (it doesn't go through it and returns with code 1 as expected, except the image is just a tiny black square), I use zsh for my shell, but the shebang is the same as well (/usr/bin/env bash).

There doesn't seem to be any relevant change in the changelogs for kitty from 0.27.0 to 0.28.1. As for Lf, I tried in r28 (the version that worked with the previous kitty versions) and r29 and the same thing happens.

rockyzhang24 commented 1 year ago

@Kazuma-chan Hi, please check my config (https://github.com/rockyzhang24/dotfiles/tree/master/.config/lf). It works perfectly.

My kitty version is 0.28.1 and lf is the latest (installed via env CGO_ENABLED=0 go install -ldflags="-s -w" github.com/gokcehan/lf@latest). Hope it's helpful.

kazmath commented 1 year ago

@rockyzhang24 I copied your config files exactly to each file, but it still didn't work (the black square isn't there anymore tho). My installation method for lf is different from yours though, I downloaded the binary from the releases tab here in github, more specifically lf-linux-amd64.tar.gz. I don't even think I have go installed in this machine.

rockyzhang24 commented 1 year ago

@Kazuma-chan Anyway it works on my side, so it means no issues for kitty and lf. I recommend you to install lf by the command I mentioned above and see if your issue is resolved.

awerty-noob commented 1 year ago

@rockyzhang24 I use your previewer config but ended in crash once I push w button trying to enter bash with the image's preview by the side. Peek 2023-06-19 17-38 return zsh: suspended (tty output) lf once I tried to input something, kitty closed. I didn't find the reason.