Better preview script integration (targeted at image previews)

[Provessor]

I am going to leave this open as a reference for later. My progress is now at https://gitlab.com/Provessor/lfp.

PR

This is aimed at being a continuation of #208

It does currently work using icat with kitty (see these scripts) and with ueberzug (these scripts).

Still TODO:

• Fix the crash from here
• Implement better method of getting drawing dimensions over passing it through nav
• Better implementation of not previewing file if current file has changed after locking
• Stop image loading if current file is changed before completion
• Y coordinate for previews is not set properly until after first resize
• Only run previewClear() if previous selection was a regular file
• Only regenerate preview if selection has changed
• Fix :delete not doing anything on 8eccbd4 (copy/move still work as usual)
• Remove flickering with period set
• Update kitty example script to match latest progress
• Update both scripts to cache previews as @Monirzadeh suggested (here)
• Fix current directory listing not updating when pasting from a copy started in another window (not in current master, only occurs sometimes?) -- can't seem to reproduce against c61391b
• Test c61391b against all settings

Outside Scope but Would be Useful:

• Passing arguments to scripts (as mentioned here)
• Preview cache memory management
• Profiler

Problems

Caching

• With implementations such as ueberzug, the image loading, processing and previewing is all done outside lf's control. This means features such as image caching (i.e. this issue) depends entirely their implementation. Possible Fixes:
  • Move files onto a tmpfs for controlling when it is loaded into RAM -- would mean wasted RAM as these files could only be used for previews.
  • The example ueberzug scripts cache an image at the desired resolution to lower processing load on the preview method.
• Other implementations like kitty or sixel can pass image data through stdout so it can be cached. However, each implementation uses their own escape sequences meaning that lf cannot reliably detect all protocols in order to properly emit the escape sequences on redisplay. kitty can transfer image data another way which then leads to the same problem as above; sixel cannot and is entirely dependent on escape sequences. Possible Fixes:
  • An option to set what escape sequence lf is expecting -- would require the user to have some technical knowledge about what implementation they are using

Asynchronous Loading

To the best of my debugging ability, lf cannot reliably start multiple subprocesses and simultaneously read their stdout (see crash here). Synchronous loading (on a fast enough machine of course) is much more consistent and the preview will always match the currently selected file. However, when trying to display images with kitty (using kitty's icat), images will be loaded into memory, processed and then sent to the terminal emulator entirely through the subprocess which can lead to some longer load times. (note: this problem isn't present with something like ueberzug).

Possible Fixes

• A script that reads a path from a FIFO similar to how ueberzug works -- can be implemented by users for any protocol
• Each reg sent to regChan will be appended to a vector of to-be-previewed files, when lf is idle it will then pick the last item on the list and generate the preview; possibly with an option to change between synchronous and a asynchronous loading. I have not tested this yet but this may create a problem where lf constantly checks the vector when idle.
• Split regChan into two goroutines with most of the logic in app.loop() but running the previewer through a separate channel.
• Split the previewer script into a generator and a viewer to let the generator be run ahead of time to do any computationally intensive tasks such as generating images for PDFs which would normally have to be run at preview-time.

Over Generation

The preview script could be called every time the window is resized, combined with synchronous generation can mean the user will have to wait a while before lf is responsive again. Also, trying to move fast through a directory can end up slow.

Possible Fixes

• A slight delay after the last action before generating previews (similar to the second preview generation), however, this could make lf feel much slower

Changes

• Previewer script is now run as (previewer) [path] [height] [width] [x] [y] [ID], x and y are the cell coordinates for the top left corner of the previewer pane.
• New option cleaner which is run as (cleaner) [ID]. This is only executed in the following situations if the last preview was generated using the script:
  • When generating a preview for a directory or displaying a cached one
  • When showing a cached preview
• A preview is regenerated if the last time it was run resulted in a non-zero exit code, or if it didn't send any text to stdout.

This means that the previewer script is required to run the cleaner script (or equivalent) when it is executed if necessary as it is not always required. Additionally, I have provided some example scripts but they do not need to operate in a similar manner, as long as the above is satisfied.

[Provessor]

@gokcehan do you have a preference of getting the current preview dimensions apart from passing it through nav?
The only other options I can see is to pass either app or ui through to preview().. or calculating it based on the termbox data.

Also do you have a preference for checking whether the current file has changed since preview() was started; would a new channel that runs the preview when the buffer is empty suffice?

[Provessor]

@gokcehan I cannot find a cross platform way to instantly terminate a process and all sub-processes in Go; Windows has a separate method. I also could not find a proper compile-time system check, would you be happy to do a runtime check and then proceed according to the result?

I would not be able to guarantee the result on Windows -- only go off what I have read.

[Provessor]

@gokcehan I cannot find a cross platform way to instantly terminate a process and all sub-processes in Go; Windows has a separate method. I also could not find a proper compile-time system check, would you be happy to do a runtime check and then proceed according to the result?

After some testing even the "linux specific" solutions do not seem to kill the process properly. It might need to just only process the most recent file in the channel? The current problem is scrolling fast through a directory will run the preview until completion (even after Process.Kill() was called on it) and then not save the stdout to cache.

[gokcehan]

@Provessor Thanks for working on this.

As I mentioned in the other PR, nav should only hold the height variable because others are not used in nav itself (i.e. height is used to calculate the index of selected files properly in directories) and other variables are not the same in different panes. You can get the other variables when necessary from app or ui as you say.

Please also address the things I have mentioned about caching in the other PR. You should be testing your patch with period option and movements such as up or down at the top and bottom should not trigger extra previews. This requires going over such movement commands to add some checks to avoid redundant operations.

Also, have you been able to fix the crash?

Handling the previous ongoing preview if the file is changed is more tricky. Killing the process seems like an overkill but I'm not sure if there is an alternative.

[Provessor]

@gokcehan I've been unable to reproduce the crash since I changed the preview handler to a buffered channel. From the stack trace it seemed the problem had to do with multiple goroutines trying to generate a preview concurrently, the new design should completely eliminate that issue.

I made that first comment with the original design, with the new preview style it would be very simple to make previewServ a pointer receiver of app.

I was hoping for some feedback on the current design before moving onto more thorough testing and handling more edge cases, are you happy with a previewServ goroutine reading from previewChan?

About handling the previous ongoing preview, killing the process doesn't work as well as I expected as it only sends SIGKILL to the started process not any of the child processes so it won't necessarily stop any current preview generation. I've tested ranger since I made the last commit on how they handle previews and it seems to lock the ui while a preview is being generated. Do you think a blocking ui would be better in this case (to keep the preview in sync with the current selection) or maybe a setting for this?

I've been busy the last week with changes due to the current global situation, hopefully I'll be able to get back on this shortly.

[gokcehan]

@Provessor I need to go over the code properly to be able to give some feedback but I don't have much time for that. Concurrent code is difficult to reason and takes time to get right. If you ask me, this integration is not a light undertaking. I can only give this a green light if (1) it doesn't limit users not using this feature in any way, (2) it works smoothly so that we don't get new issues regarding this, (3) it doesn't add random crashes, and (4) it doesn't make the code significantly complicated to maintain. Locking the ui doesn't sound good in regards to (1). Caching will most likely be a problem in regards to (2). In general it is not a good idea add new goroutines randomly in the code in regards to (3) and (4). New goroutines should likely be integrated to app.loop somehow.

[Provessor]

@gokcehan Ok; I'll move all the logic into app.loop(), play around with the handling until it works well all the edge cases are handled properly and let you know when I think it satisfies all of those.

[Provessor]

@Monirzadeh I took a minute and the ueberzug example scripts now caches previews very similar to ranger. It could use some improvement as everything is done by the image width so images previews can get produced that are a higher resolution than what is needed.
Also, I am unsure if ranger takes care of the cache size however this can be taken care of here with cronjob that trims the ~/.cache/lf directory.

[Provessor]

@gokcehan could I get your opinion on some of the listed problems? Mainly about the asynchronous preview generation, simply wrapping the generation logic in a goroutine gives me the same crash that you mentioned on the previous PR.

Also, would you be interested in what I've listed as possible outside scope? If so, would you like these in a separate PR?

This is just an idea but how would you feel about instead of setting a preview script, registering preparation commands (either based on extension or mimetype) and a alternate preview command if lf can't just print the stdout. This would solve #322 and allow lf to handle more advanced preview caching and asynchronous generation internally without locking it into a specific protocol; but this would add quite a bit more logic and complicate the generation process.

I have looked through the ranger preview logic, and this seems to be at feature parity (with my ueberzug example scripts) and suffers from all of the same problems (except from the startup delay here which could be just due to a slow preview script) if you're most interested in that for readiness.

[Monirzadeh]

@Monirzadeh I took a minute and the ueberzug example scripts now caches previews very similar to ranger. It could use some improvement as everything is done by the image width so images previews can get produced that are a higher resolution than what is needed.
Also, I am unsure if ranger takes care of the cache size however this can be taken care of here with cronjob that trims the ~/.cache/lf directory.

@Provessor i build lf and download scripts (copy to ~/.config/lf/ and make them executable)
my lfrc has only this line

set preview true
set previewer ~/.config/lf/previewer
set cleaner ~/.config/lf/cleaner

startlf run lf but image preview not showing
~/.cache/lf is exsit , but it is empty
UPDATE 1:
image preview generate for movie but files has problem and they are not show in image viewer.
UPDATE 2:
for sure i test uberzug with this example and it is work fine
side note: uberzug not work on zsh. i run startlf with bash and zsh but nothing change

[Provessor]

@Monirzadeh what OS is this on?

The updated startlf script creates the $HOME/.cache/lf folder if it doesn't exist on launch so you can test whether that works by removing it before you run startlf. Also, this new version uses (POSIX) sed to create a unique filename and ImageMagick to shrink the image for a faster preview time so those must be available.

The preview depends on the cache so if that doesn't exist, you won't get any preview.

[Monirzadeh]

@Monirzadeh what OS is this on?

The updated startlf script creates the $HOME/.cache/lf folder if it doesn't exist on launch so you can test whether that works by removing it before you run startlf. Also, this new version uses (POSIX) sed to create a unique filename and ImageMagick to shrink the image for a faster preview time so those must be available.

The preview depends on the cache so if that doesn't exist, you won't get any preview.

my os is lubuntu 20.04
i install ImageMagick and (sed installed) but i don't get image or video preview
and get new scripts for startlf
i get this error if i exit from lf while select a video or image

/home/myusername/.local/bin//startlf: line 28: 28963 Illegal instruction     (core dumped) ueberzug layer --parser bash --silent < "${UEBERZUG_FIFO}"

it doesn't work. did i miss something?

[Provessor]

@Monirzadeh ah that seems to be a problem with ueberzug, but it was working fine before?

Does this script work (from the ueberzug examples)?

#!/usr/bin/env bash
source "`ueberzug library`"

ImageLayer 0< <(
    ImageLayer::add [identifier]="example0" [x]="0" [y]="0" [path]="/path/to/some/image.png"
    sleep 5
    ImageLayer::remove [identifier]="example0"
    sleep 5
)

If you replace the /path/to/some/image.png with an image it should add (display) an image for 5 seconds and then remove it and wait another 5 seconds before exiting.

EDIT: I had a quick look through the ueberzug issues and saw this https://github.com/seebye/ueberzug/issues/77, did you install ueberzug through pip or the ubuntu repositories?

[Monirzadeh]

@Monirzadeh ah that seems to be a problem with ueberzug, but it was working fine before?

i have a new clean install of lubuntu 20.04.
before that it work fine

Does this script work (from the ueberzug examples)?

yes it show image without problem

EDIT: I had a quick look through the ueberzug issues and saw this seebye/ueberzug#77, did you install ueberzug through pip or the ubuntu repositories?

i install uberzug through pip with pillow-simd. should i install uberzug with pillow instead of pillow-simd?
pillow-simd can be related?

UPDATE:
i switch from pillow-simd to pillow and problem solved.
it is a little weird . both of them work fine with script that you send. it just stop working when integrate with image preview. it is a problem of impv or pillow-simd.
i prefer pillow-simd because of better performance.
if it Possible for you please add this to Problems to solve later.
i use new version and report any bug that i see.
thanks for your time i am happy to test your work.

[Provessor]

@Monirzadeh it's a known problem that ueberzug will crash on some machines with pillow-simd when resizing. The best solution for this would be to get the perfect size in the script, you will probably need to change the TEXT_WIDTH variable in startlf so it is the exact with of a character (in pixels) in your terminal.

I just pushed an update to the ueberzug example scripts to ensure the images are always cached with the correct extension (it was always .png before) which could possibly solve your problem.

As a side note, can you see anything in ~/.cache/lf now? In my script those images are created before anything is sent to ueberzug so that problem shouldn't have affected what's in that directory.

EDIT: the scripts now use ImageMagick to cache the images at the perfect size so if the TEXT_WIDTH variable is set to exactly match, I believe pillow(-simd) would go unused from ueberzug as it is only used for image manipulation.

[Tanish2002]

@Provessor I tried to use your fork with the scripts you have provided.. I can confirm that image previews work perfectly for me..It's just that lf lags when previewing the image for first time but after it has created the cache it works flawlessly.

For a Wallpaper folder of 86M.. lf created a cache folder of size 6.4M in ~/.cache/lf

❯ du -sh ~/Wallpapers/
86M     /home/tanish2002/Wallpapers/

❯ du -sh ~/.cache/lf
6.4M    /home/tanish2002/.cache/lf/

[Provessor]

@Tanish2002 Thanks for your feedback! Those scripts sacrifice first-time viewing performance in order to get the best performance later-on, this could be changed with a little bash knowledge if you would prefer it the other way around. I don't know your background but I would be happy to help if you want to change those scripts in a certain way.

[06kellyjac]

This branch is running pretty well on my machine using your ueberzug scripts in both Alacritty & Alacritty + tmux.

I'm tweaking the scripts here and there but it's a good baseline. The speed to cache is a bit slow but loading images from the cache is pretty quick.
I've togged off the caching for now by commenting it out & passing the file name to draw_image & it runs nice and fast.
There's a slight lag waiting for images to catch up but I have full freedom to control lf while it does that

[Monirzadeh]

@Provessor new update is pretty well compare to the previous version. i test that for 2 days and confront some problem and some suggestion for improvement.
last method force me to comeback to the ranger, but this version change the game :) ❤️

major problem

1. first time you open new folder you have a lag to generate thumbnail but after that every things smooth 👍 . when i reset my computer and open that folder, lag happen again. i don't know why . all cache present in '~/.cache/lf' so lf shouldn't try to regenerate them so what happen in background?
2. ranger better manage that lag time. lf ui freeze on thumbnail generate phase , ranger is more smoother ui while generate thumbnail. lf is great after generate thumbnail finish . i am not sure what is the exact different or it can be improve or not? (i test that on folder contain more than 200 video so this lag can be annoying) but it acceptable sacrifice
   lf how Handel multiple file generation? do all of them at once? i don't read ranger source code completely but i think it is make a list of work and prioritize them. (may be i wrong about that)
   lf is 8-12% lower cpu usage in thumbnail generate phase 👍 but why ui freeze on that badly? 🤔 (i cheek average usage with htop not a logger)
3. is it possible to separate process lf ui and generate thumbnail step from each other. i am not so much familiar with multi processing ,but i know Firefox processing ui element on separate core and thread for an smoother response time. this article explain the concept.
4. i see a weird behavior on my NAS. it has so much lag (some time 2 or 3 second to switch next file ) while image preview is active. it not related to thumbnail generation phase. new preview script generate thumbnail on my disk (and not call them from NAS ). so why this lag happen after that?
5. i run lf (instead of startlf) to remove show preview step with ueberzug. this lag still happen so this delay is not related to ueberzug show the image.
6. some time thumbnail generate multiple size for same file. i don't have any idea about that. they have @65 or @15 ,etc at end of file name. something like this.

!path!to!file!file.mkv
!path!to!file!file.mkv@35
!path!to!file!file.mkv@65

7. i think it is better to not cache image - kinda unnassery step. just one exception ( cache image can be helpful For NAS with slow connection in general) .
   do you think show large image with ueberzug take more time than generate thumbnail + show a smaller image? it can be measure with logger we talked about.

minor problem

8. i have this line in my lfrc set period 1, if i select a text preview is fine. but if it a video or image a highlighted text 'LOADING' flashing under the image ( you see a thin line in up lef of image preview. it is annoying if it posible remove that for preview file aftter tumbnail load). at the first i think it is not a big deal but when i test lf on my NAS i experience a constant lag not just for image preview - explained on number 5 and 6 - it is not related to ueberzug preview if i run that with lf it is happen but if you disable image preview this lag gone. maybe something unnecessary reload over and over in background while lf reload.
9. it is better if change ffmpegthumbnailer -t 0% to ffmpegthumbnailer -t 10% so many video start with black screen or a has same intro so user see black or same image for different video.
10. i change ffmpegthumbnailer -q 10 but image quality is a little low compare to old version. it is not a huge problem but why? is it because of image preview size added to the script?

@Monirzadeh it's a known problem that ueberzug will crash on some machines with pillow-simd when resizing. The best solution for this would be to get the perfect size in the script, you will probably need to change the TEXT_WIDTH variable in startlf so it is the exact with of a character (in pixels) in your terminal.
I just pushed an update to the ueberzug example scripts to ensure the images are always cached with the correct extension (it was always .png before) which could possibly solve your problem.
As a side note, can you see anything in ~/.cache/lf now? In my script those images are created before anything is sent to ueberzug so that problem shouldn't have affected what's in that directory.
EDIT: the scripts now use ImageMagick to cache the images at the perfect size so if the TEXT_WIDTH variable is set to exactly match, I believe pillow(-simd) would go unused from ueberzug as it is only used for image manipulation.

do you want me test ueberzug with pillow-simd? is that the reason you cache the image?
Sorry to Bother You with this long review. this update is a huge improvement and now i can comeback to lf again.
Thanks for your effort 🥇

UPDATE : after i post this massage i open a folder contain so many video ( a complete nightmare with past method) after ui freeze and acceptable lag it's work so good. i can't resist to not send you another 😃 👍 .

[Provessor]

@Monirzadeh thanks for you list of improvements, I'll have a better look when I'm free next (in a few days) but to try to help with a couple of your issues:

when i reset my computer and open that folder, lag happen again

Is the terminal running lf at the same size as before? My scripts will cache an image for each size. This sounds like a problem with my scripts

ranger better manage that lag time. lf ui freeze on thumbnail generate phase

Possibly because they offload as much as possible to ueberzug? It might be better for these scripts to do what was mentioned #304 (comment), I'll do some testing on this when I have time.

i don't read ranger source code completely but i think it is make a list of work and prioritize them

I'm also not sure on this, I have listed this in OP under possible fixes for a lack of asynchronous loading but I don't want to spend time on any of these until I get feedback from @gokcehan. Ideally, previewing would have an option to run everything asynchronously but this would disrupt some of the current source code structure.

some time thumbnail generate multiple size for same file

This is the script caching an image at the exact correct size for what lf needs; ranger caches pdf and video images in the same way. It is related to the hash_filename function in the scripts and can be changed to match each use case. The problem it is trying to solve is saving processing when only a low resolution is needed but yes it will cause the scripts to save many different sizes.

i see a weird behavior on my NAS. it has so much lag

i run lf (instead of startlf) to remove show preview step with ueberzug. this lag still happen so this delay is not related to ueberzug show the image.

What if you disable the preview script or preview entirely?

i think it is better to not cache image - kinda unnassery step

cache image can be helpful For NAS with slow connection in general

This is dependent on the scripts. I may change some caching with my example scripts but ideally when this is merged there would be a few examples on the wiki (only caching NAS files could be one of them) and a script could be modified to only cache images under a certain folder, etc.

I understand that's not everything you brought up, but as I said above, I'll have a better look when I'm free next.

[Provessor]

@06kellyjac Thanks for your feedback! What you changed might be a better default in the example scripts (and IIRC what ranger does which seems to be what most people want :) ).

[Monirzadeh]

Is the terminal running lf at the same size as before? My scripts will cache an image for each size. This sounds like a problem with my scripts

yes it happen at same size. why lf need to generate different size? ueberzug can get size and preview on that size.is it for the performance?
UPDATE: i browse a folder with lf in full screen (to generate thumbnail). after that open another lf side by side (split screen) as you mention it try to generate thumbnail for new lf size again :(

What if you disable the preview script or preview entirely?

it is so much smoother

This is dependent on the scripts. I may change some caching with my example scripts but ideally when this is merged there would be a few examples on the wiki (only caching NAS files could be one of them) and a script could be modified to only cache images under a certain folder, etc.

thanks it can be a very useful option for me

[06kellyjac]

The convert is done based on the current size lf (with this pr) sends to the script, times your text width,
convert "$1" -resize "$(expr ${TEXT_WIDTH} \* $3)"x "${TMP_FILE}"
This was likely done to avoid caching images larger than they have to & also speed up rendering. e.g. you dont cache the whole 8k image and try render the whole 8k image in a small 400px size.

IDK how other systems like ranger do caching but maybe it'd be best to do caching at a couple different increments so resizing the term/lf by a couple pixels doesn't require a fresh cache

---

Edit:

Looks like ranger just runs convert without a resize, renders svgs & other vector style stuff to 1920x1080 by default

https://github.com/ranger/ranger/blob/081e73152a9391211770fab56676d7d974413ae6/ranger/data/scope.sh#L121

This is all preview script behavior so people are free to change it how they see fit.
Nice to have a good default though

[Monirzadeh]

i test a new condition on my NAS.
if i select ( just select noting else) a video or image (need image preview) whole lf slow down (press : take 3 to 5 second until relay go to command mode)
but if i select a text file it work smooth (press : go to command mode instantly)

[Provessor]

@Monirzadeh

i select ( just select noting else) a video or image (need image preview) whole lf slow down (press : take 3 to 5 second until relay go to command mode)
but if i select a text file it work smooth (press : go to command mode instantly)

This is most likely due to the images being much larger. I'm not sure how FFmpeg produces video previews but chances are it has to load some significant portion of the video. This isn't really something that could be fixed outside of the generator/viewer idea.

What if you disable the preview script or preview entirely?

it is so much smoother

Is this without any previews or using the built-in? I've stated in OP that to fix some crashes (and comply with gokechan's requests) I had to make the previewing done synchronously. I could possibly make the built-in run asynchronously again easily but it would be much better for one solution that covers both.

it is better if change ffmpegthumbnailer -t 0% to ffmpegthumbnailer -t 10% so many video start with black screen or a has same intro so user see black or same image for different video.

This would be personal preference, I believe the default is ~10% but I chose 0% in the example to try and make it faster. Maybe I should leave that on the defaults.

i change ffmpegthumbnailer -q 10 but image quality is a little low compare to old version. it is not a huge problem but why? is it because of image preview size added to the script?

With the current example scripts that's probably because the TEXT_WIDTH variable is too small for your screen, for an immediate solution you should try increasing it (in startlf).

i have this line in my lfrc set period 1, if i select a text preview is fine. but if it a video or image a highlighted text 'LOADING' flashing under the image ( you see a thin line in up lef of image preview. it is annoying if it posible remove that for preview file aftter tumbnail load). at the first i think it is not a big deal but when i test lf on my NAS i experience a constant lag not just for image preview - explained on number 5 and 6 - it is not related to ueberzug preview if i run that with lf it is happen but if you disable image preview this lag gone. maybe something unnecessary reload over and over in background while lf reload.

The period setting redraws the whole ui and the "loading" flashes under the images while the preview script is running again. This could only be removed by making lf not generate previews on that timer (needs a slight refactor for that).

@06kellyjac do you think I should change the script to match ranger or just stop caching images at specific sizes? Or maybe something else?

As a side-note, if I don't get any feedback from @gokcehan in the near future about the direction they want this to take, I would be willing to pick (in my opinion) the best solutions from OP and implement those without worrying about whether or not it would get merged.

[Monirzadeh]

@gokcehan any update on this PR?

[Provessor]

As I just added to OP, I am going to continue this at https://gitlab.com/Provessor/lfp (so please give any issues or feedback over there). I am a bit disappointed at the lack of feedback or any activity on the issues, but luckily this is open source so I can fork. @gokcehan if you still want this or anything else I do to be merged than let me know; I would rather not have two projects for this.

P.S. thanks for your feedback everyone

[gokcehan]

@Provessor Sorry, I have been quite busy lately and haven't been able to spare time on lf. I have been trying to catch up with issues and PRs in the last couple of days. I couldn't get back to this yet because there seems to be a lot of information here. I'm sorry to hear you are disappointed because that was not my intention.

Regarding this PR, I have already tried this in #208. I have also reviewed your PR once before. As I mentioned, this PR takes time to review and I don't have time and motivation to do it repeatedly. People still seems to be reporting some issues here so I have my doubts about the stability. I don't want to spend an afternoon trying to set this up to come across with similar issues we have talked about before. I have already told you about my criteria if you want see this getting merged. Essentially, it should be relatively stable and it should not effect others not using this feature in a significant way.

I understand you need some feedback about the direction of this PR. You have been working on this for a while and you are likely much more familiar than me about the requirements of this PR. I need to spend time on this to be able to give you such feedback. Unfortunately, I don't share the same enthusiasm as you about this feature. Images in terminals would have been a cool feature but there is no standard for it and all existing implementations feel to me like hacks. I don't expect this to change any time soon. I find external programs to be cleaner and more viable solutions to deal with images. There are different problems in lf that I think are more important than image previews. I would rather spend my time, if any, working on these.

Although I can't spare time working on issues, I'm trying my best to keep up with PRs. I think that is the essential role of a maintainer. We have merged many PRs so far. In fact, most of the changes we have in the last couple of releases were made by contributors other than me. The kind of patches that are likely to be quickly merged are those that are small and simple. If your patch touches almost all mechanisms in the code, that is usually not a good sign. These patches should rather be divided into smaller incremental changes. For example, I feel like this PR could have been split into smaller PRs. One PR could have added parameters to the previewer script, another could have introduced an option for a cleaner script, a third could have changed up/down commands to no-op for when they are at the top and bottom and so on.

There may be cases where this strategy falls short for management. Not every feature can be implemented in small incremental steps. I also have some criteria for new features that may be too hard for this feature like not having hard coded external program dependency in the code, or not effecting the rest of the code. If you feel like these are limiting the progress, then forking is the way to go.

I would like to see more code contributors around. Unfortunately, forks tend to reduce that. But sometimes these things happen. In a way, this is the beauty of open source. I didn't want to create an organization namespace for lf, because I didn't want people to consider this as the one true fork. This is simply a fork I try to maintain in the way I think is for the best. I wish you the best of luck with your fork.

If you feel like you can continue this PR without getting any technical feedback from me, then feel free to do so. If at any point, you are confident about this PR being ready, then ping me again so I can test. However, I see in your fork that you list some other changes such as termbox to tcelll refactoring and a new server/client implementation. If this PR becomes a giant blob of changes with all those features just to implement image previews, then I'm afraid I don't think it will be merged.

[Provessor]

@gokcehan Thank you for taking the time to reply to this. I do like your approach to how lf should be written, I think it will lead to a very maintainable and long lasting codebase so I will try to keep any changes I make in that essence.

The fact is at the moment I have quite a bit of free time and would much rather be spending that working on lf rather than waiting for some feedback from a very busy maintainer. It sounds like we would both like to avoid splitting the effort so if you are interested in the changes I listed, I could submit a PR for each when I think they are complete. The reasoning for this is to distance my work from here so I am no longer worrying about merging or getting feedback before I can continue which I think would let me get through these much faster, and then if you wish for a PR, make reviewing faster because the review is no longer about what might be.

So currently I will finish my work and then contact you with what I have done (I have not seen an email anywhere so possibly a smaller PR?). I don't think the comments of a PR is the right place to be discussing this so feel free to contact me another way if you want to keep discussing this.

[gokcehan]

@Provessor Sure, that sounds reasonable. I'm aware even the most simple thing takes a lot of time when it requires back and forth communication with me. Keep working on your fork and let's see how it goes.

We have an inactive google groups mailing list here. Maybe we can use it for discussion. I think you can find my email there as well.

[Provessor]

@gokcehan Perfect, I'm closing this then.