search

hackerb9 / lsix

Like "ls", but for images. Shows thumbnails in terminal using sixel graphics.
GNU General Public License v3.0
4.01k stars 127 forks source link

Bugfix/no decode delegate for this image format #31

Closed starox closed 4 years ago

starox commented 4 years ago

This adds all file formats known by ImageMagick and fix directory listing. The file format check is expensive, but adds supports for movies as well. Movies montage are extremely slow though ... and I had no patience to test it For listing with arguments, only the first image of a multiple image format is shown. The old behaviour without arguments is kept.

hackerb9 commented 4 years ago

Thank you, Starox. These mostly look like good changes, but I'm not sure about the way you added directory support (cfb318d) and the reason for the extra echo (e52c4e3).

The directory support seems overly complicated. Why is it using find? Also, when I try it out on directories, I get lots of "no decode delegate" errors because it is in effect replacing lsix directory with lsix directory/*. What directory support should do is something more akin to cd directory; lsix; cd - .

starox commented 4 years ago

Thanks. I forget some people's editors can't handle tabs.

I updated only the part which interested me. When I saw that the entire script was mixing tabs, space, and 2 indention style for if and for, I let it aside.

starox commented 4 years ago

Thank you, Starox. These mostly look like good changes, but I'm not sure about the way you added directory support (cfb318d) and the reason for the extra echo (e52c4e3).

The directory support seems overly complicated. Why is it using find? Also, when I try it out on directories, I get lots of "no decode delegate" errors because it is in effect replacing lsix directory with lsix directory/*. What directory support should do is something more akin to cd directory; lsix; cd - .

Thank you for taking time to review it. Most of the answers are on individual comments above. I am confident of the directory behaviour because I compared with the ouput of 'ls'

There is one thing which embarass me is the behiaviour of lsix to show or not multiple images of the same file. Because the way I build the file list. When I read again my code I think I inverted the meaning in my head and I must have disable the feature to show all images of a gif.

I think having a special flag like -a to show all images or only the first one would be better.

hackerb9 commented 4 years ago

Let's separate this into two problems then: directory behavior and expanding GIFs.

Directory behavior: I'll take a look at adding the fix I mentioned in the comments above.

I think having a special flag like -a to show all images or only the first one would be better.

That would be good, if we can find a way to do it simply and succinctly. Please submit a new patch for just that and we'll see what can be done.