gokcehan / lf

Terminal file manager
MIT License
7.63k stars 325 forks source link

Preview window colors #1743

Closed araaha closed 2 months ago

araaha commented 3 months ago

When using bat to preview a file with syntax highlighting, lf's preview window uses different colors compared to bat. This started happening with r32.

2024-06-05-13:20:06-screenshot 2024-06-05-13:20:14-screenshot

GrzegorzKozub commented 3 months ago

I seem to have the same issue using lf with kitty and tmux.

I describe it as slight blue and/or red tint when previewing with bat from lf. Also present when not using tmux. I also have map i $~/.config/lf/previewer $f binding and the tint appears there as well.

This tint is not present when using bat directly, still with kitty and tmux . Also, there's no tint on the lf dir/file list.

image

My previewer simply says bat --force-colorization --paging=never --plain "$1" || true . No combination of bat options seems to help here.

GrzegorzKozub commented 3 months ago

I investigated a bit more on Windows. There, the color tint was a problem for bat alone, without lf in the picture. I noticed that Windows Terminal does not set the $COLORTERM whereas WezTerm does set it to truecolor. Sure enough, the problem did not appear on WezTerm. So I worked around it by explicitly setting $COLORTERM to truecolor when running on Windows Terminal https://github.com/GrzegorzKozub/pwsh/commit/a76c131e9d63114222a4882259bae4b667e2e435.

GrzegorzKozub commented 3 months ago

Back on Linux, $COLORTERM is always set to truecolor for me. That's how I configured tmux. So it wasn't that.

I noticed the following behavior regarding $TERM though:

In both these cases bat displays the incorrect, tinted colors when running from lf.

When I explicitly set $TERM to xterm-256color then the issue is fixed and bat displays the same colors inside lf as when launched alone.

So in the end, my workaround is:

alias lf='TERM=xterm-256color lf' # fix bat color tint

https://github.com/GrzegorzKozub/dotfiles/commit/39de43a8c5b9be77352a22e095250825e5416586

Specific to lf. I still think we should fix the issue in lf.

joelim-work commented 3 months ago

@GrzegorzKozub Thank you for investigating. It sounds like the tint effect you describe is due to whether bat is running in truecolor mode or not. You should be able to verify this by piping the output of bat into a hexdump tool and inspecting the terminal sequences used.

That being said, I don't think it makes sense to make any changes to lf. AFAIK lf does not manipulate the TERM/COLORTERM environment variables, which means the output of bat simply depends on the environment it was run in.

For your workaround, maybe it makes more sense to define TERM when invoking bat in the previewer script, not when running lf. It's probably also worth considering raising an issue with bat to see why it (or its crates) don't work with certain values for TERM.

GrzegorzKozub commented 3 months ago

You see when we take lf out of the picture completely then bat works fine alone. No workarounds needed.

Here's a summary of what fixes the colors when previewing in lf and what doesn't. Sorry for being verbose but I just want the test results to be understood.

So clearly something to do with lf. I have my workaround so I'm not forcing the fix. Up to you guys.

joelim-work commented 3 months ago

Actually, it's good to be verbose. From the sounds of your description it seems like bat is not relevant to the issue at all, and we should be trying to eliminate it from the reproduction steps, to make it as minimal as possible.

Are you able to reproduce the tint issue if you:

  1. Run bat outside of lf and save the output into a file
  2. Change the previewer script to simply cat the contents of that same file

And if so, can you also provide a minimal hexdump of that file. Ideally the reproduction steps can be minimized to just having a single echo statement (output a colored string) in the previewer script, where changing TERM gives a different result.

BTW lf doesn't handle output colors directly - it uses the tcell TUI library to do so. At this stage my guess is that lf converts escape sequences to tcell.Style objects, which is converted back differently when actually writing to the terminal.

GrzegorzKozub commented 3 months ago

Removed all the TERM workarounds and left it at tmux-256color as set by tmux.

Did bat dnd.sh > bat-output.txt. Yields bat-output.txt.

Then, used cat in the previewer. Yields:

image

Don't quite follow your request regarding the hexdump, sorry :)

joelim-work commented 3 months ago

You need to use the same settings for bat as you do in the previewer script. So:

bat --force-colorization --paging=never --plain dnd.sh > bat-output.txt

Simply bat dnd.sh > bat-output.txt isn't sufficient because bat outputs the regular file contents like cat without any additions when not writing directly to the terminal, see https://github.com/sharkdp/bat?tab=readme-ov-file#file-concatenation

As for the hexdump, don't worry about it, just upload the bat-output.txt directly as before and I can examine it myself.

GrzegorzKozub commented 3 months ago

Ah, of course. Now we're getting somewhere. The updated bat-output.txt contains color escape sequences.

image

joelim-work commented 3 months ago

OK so the format of bat-output.txt is what I expected. The hexdump (using hexdump -C) looks like this:

Click to expand ``` 00000000 1b 5b 33 3b 33 38 3b 32 3b 31 34 36 3b 31 33 31 |.[3;38;2;146;131| 00000010 3b 31 31 36 6d 23 1b 5b 30 6d 1b 5b 33 3b 33 38 |;116m#.[0m.[3;38| 00000020 3b 32 3b 31 34 36 3b 31 33 31 3b 31 31 36 6d 21 |;2;146;131;116m!| 00000030 2f 75 73 72 2f 62 69 6e 2f 65 6e 76 20 62 61 73 |/usr/bin/env bas| 00000040 68 1b 5b 30 6d 0a 0a 1b 5b 33 38 3b 32 3b 32 31 |h.[0m...[38;2;21| 00000050 32 3b 31 39 30 3b 31 35 32 6d 67 73 65 74 74 69 |2;190;152mgsetti| 00000060 6e 67 73 1b 5b 30 6d 1b 5b 33 38 3b 32 3b 31 36 |ngs.[0m.[38;2;16| 00000070 39 3b 31 38 32 3b 31 30 31 6d 20 73 65 74 20 6f |9;182;101m set o| 00000080 72 67 2e 67 6e 6f 6d 65 2e 64 65 73 6b 74 6f 70 |rg.gnome.desktop| 00000090 2e 6e 6f 74 69 66 69 63 61 74 69 6f 6e 73 20 73 |.notifications s| 000000a0 68 6f 77 2d 62 61 6e 6e 65 72 73 20 66 61 6c 73 |how-banners fals| 000000b0 65 1b 5b 30 6d 0a 0a 1b 5b 33 38 3b 32 3b 32 31 |e.[0m...[38;2;21| 000000c0 32 3b 31 39 30 3b 31 35 32 6d 67 64 62 75 73 1b |2;190;152mgdbus.| 000000d0 5b 30 6d 1b 5b 33 38 3b 32 3b 31 36 39 3b 31 38 |[0m.[38;2;169;18| 000000e0 32 3b 31 30 31 6d 20 6d 6f 6e 69 74 6f 72 1b 5b |2;101m monitor.[| 000000f0 30 6d 1b 5b 33 38 3b 32 3b 32 31 32 3b 31 39 30 |0m.[38;2;212;190| 00000100 3b 31 35 32 6d 20 2d 1b 5b 30 6d 1b 5b 33 38 3b |;152m -.[0m.[38;| 00000110 32 3b 32 31 32 3b 31 39 30 3b 31 35 32 6d 79 1b |2;212;190;152my.| 00000120 5b 30 6d 1b 5b 33 38 3b 32 3b 32 31 32 3b 31 39 |[0m.[38;2;212;19| 00000130 30 3b 31 35 32 6d 20 2d 1b 5b 30 6d 1b 5b 33 38 |0;152m -.[0m.[38| 00000140 3b 32 3b 32 31 32 3b 31 39 30 3b 31 35 32 6d 64 |;2;212;190;152md| 00000150 1b 5b 30 6d 1b 5b 33 38 3b 32 3b 31 36 39 3b 31 |.[0m.[38;2;169;1| 00000160 38 32 3b 31 30 31 6d 20 6f 72 67 2e 66 72 65 65 |82;101m org.free| 00000170 64 65 73 6b 74 6f 70 2e 6c 6f 67 69 6e 31 1b 5b |desktop.login1.[| 00000180 30 6d 1b 5b 33 38 3b 32 3b 32 31 32 3b 31 39 30 |0m.[38;2;212;190| 00000190 3b 31 35 32 6d 20 1b 5b 30 6d 1b 5b 33 38 3b 32 |;152m .[0m.[38;2| 000001a0 3b 32 33 31 3b 31 33 38 3b 37 38 6d 7c 1b 5b 30 |;231;138;78m|.[0| 000001b0 6d 1b 5b 33 38 3b 32 3b 32 31 32 3b 31 39 30 3b |m.[38;2;212;190;| 000001c0 31 35 32 6d 20 1b 5b 30 6d 1b 5b 33 38 3b 32 3b |152m .[0m.[38;2;| 000001d0 32 31 32 3b 31 39 30 3b 31 35 32 6d 5c 1b 5b 30 |212;190;152m\.[0| 000001e0 6d 0a 1b 5b 33 38 3b 32 3b 32 31 32 3b 31 39 30 |m..[38;2;212;190| 000001f0 3b 31 35 32 6d 20 20 1b 5b 30 6d 1b 5b 33 38 3b |;152m .[0m.[38;| 00000200 32 3b 32 31 32 3b 31 39 30 3b 31 35 32 6d 67 72 |2;212;190;152mgr| 00000210 65 70 1b 5b 30 6d 1b 5b 33 38 3b 32 3b 32 31 32 |ep.[0m.[38;2;212| 00000220 3b 31 39 30 3b 31 35 32 6d 20 2d 2d 1b 5b 30 6d |;190;152m --.[0m| 00000230 1b 5b 33 38 3b 32 3b 32 31 32 3b 31 39 30 3b 31 |.[38;2;212;190;1| 00000240 35 32 6d 6c 69 6e 65 2d 62 75 66 66 65 72 65 64 |52mline-buffered| 00000250 1b 5b 30 6d 1b 5b 33 38 3b 32 3b 32 31 32 3b 31 |.[0m.[38;2;212;1| 00000260 39 30 3b 31 35 32 6d 20 2d 1b 5b 30 6d 1b 5b 33 |90;152m -.[0m.[3| 00000270 38 3b 32 3b 32 31 32 3b 31 39 30 3b 31 35 32 6d |8;2;212;190;152m| 00000280 69 1b 5b 30 6d 1b 5b 33 38 3b 32 3b 31 36 39 3b |i.[0m.[38;2;169;| 00000290 31 38 32 3b 31 30 31 6d 20 1b 5b 30 6d 1b 5b 33 |182;101m .[0m.[3| 000002a0 38 3b 32 3b 31 36 39 3b 31 38 32 3b 31 30 31 6d |8;2;169;182;101m| 000002b0 22 1b 5b 30 6d 1b 5b 33 38 3b 32 3b 31 36 39 3b |".[0m.[38;2;169;| 000002c0 31 38 32 3b 31 30 31 6d 7b 27 4c 6f 63 6b 65 64 |182;101m{'Locked| 000002d0 48 69 6e 74 27 3a 20 3c 66 61 6c 73 65 3e 7d 1b |Hint': }.| 000002e0 5b 30 6d 1b 5b 33 38 3b 32 3b 31 36 39 3b 31 38 |[0m.[38;2;169;18| 000002f0 32 3b 31 30 31 6d 22 1b 5b 30 6d 1b 5b 33 38 3b |2;101m".[0m.[38;| 00000300 32 3b 32 31 32 3b 31 39 30 3b 31 35 32 6d 20 1b |2;212;190;152m .| 00000310 5b 30 6d 1b 5b 33 38 3b 32 3b 32 33 31 3b 31 33 |[0m.[38;2;231;13| 00000320 38 3b 37 38 6d 7c 1b 5b 30 6d 1b 5b 33 38 3b 32 |8;78m|.[0m.[38;2| 00000330 3b 32 31 32 3b 31 39 30 3b 31 35 32 6d 20 1b 5b |;212;190;152m .[| 00000340 30 6d 1b 5b 33 38 3b 32 3b 32 31 32 3b 31 39 30 |0m.[38;2;212;190| 00000350 3b 31 35 32 6d 5c 1b 5b 30 6d 0a 1b 5b 33 38 3b |;152m\.[0m..[38;| 00000360 32 3b 32 31 32 3b 31 39 30 3b 31 35 32 6d 20 20 |2;212;190;152m | 00000370 1b 5b 30 6d 1b 5b 33 38 3b 32 3b 32 33 34 3b 31 |.[0m.[38;2;234;1| 00000380 30 35 3b 39 38 6d 77 68 69 6c 65 1b 5b 30 6d 1b |05;98mwhile.[0m.| 00000390 5b 33 38 3b 32 3b 32 31 32 3b 31 39 30 3b 31 35 |[38;2;212;190;15| 000003a0 32 6d 20 1b 5b 30 6d 1b 5b 33 38 3b 32 3b 31 36 |2m .[0m.[38;2;16| 000003b0 39 3b 31 38 32 3b 31 30 31 6d 72 65 61 64 1b 5b |9;182;101mread.[| 000003c0 30 6d 1b 5b 33 38 3b 32 3b 32 33 31 3b 31 33 38 |0m.[38;2;231;138| 000003d0 3b 37 38 6d 3b 1b 5b 30 6d 1b 5b 33 38 3b 32 3b |;78m;.[0m.[38;2;| 000003e0 32 31 32 3b 31 39 30 3b 31 35 32 6d 20 1b 5b 30 |212;190;152m .[0| 000003f0 6d 1b 5b 33 38 3b 32 3b 32 33 34 3b 31 30 35 3b |m.[38;2;234;105;| 00000400 39 38 6d 64 6f 1b 5b 30 6d 0a 1b 5b 33 38 3b 32 |98mdo.[0m..[38;2| 00000410 3b 32 31 32 3b 31 39 30 3b 31 35 32 6d 20 20 20 |;212;190;152m | 00000420 20 1b 5b 30 6d 1b 5b 33 38 3b 32 3b 32 31 32 3b | .[0m.[38;2;212;| 00000430 31 39 30 3b 31 35 32 6d 67 73 65 74 74 69 6e 67 |190;152mgsetting| 00000440 73 1b 5b 30 6d 1b 5b 33 38 3b 32 3b 31 36 39 3b |s.[0m.[38;2;169;| 00000450 31 38 32 3b 31 30 31 6d 20 73 65 74 20 6f 72 67 |182;101m set org| 00000460 2e 67 6e 6f 6d 65 2e 64 65 73 6b 74 6f 70 2e 6e |.gnome.desktop.n| 00000470 6f 74 69 66 69 63 61 74 69 6f 6e 73 20 73 68 6f |otifications sho| 00000480 77 2d 62 61 6e 6e 65 72 73 20 66 61 6c 73 65 1b |w-banners false.| 00000490 5b 30 6d 0a 1b 5b 33 38 3b 32 3b 32 31 32 3b 31 |[0m..[38;2;212;1| 000004a0 39 30 3b 31 35 32 6d 20 20 1b 5b 30 6d 1b 5b 33 |90;152m .[0m.[3| 000004b0 38 3b 32 3b 32 33 34 3b 31 30 35 3b 39 38 6d 64 |8;2;234;105;98md| 000004c0 6f 6e 65 1b 5b 30 6d 0a 0a |one.[0m..| 000004c9 ```

This contains true color escape sequences in the form ESC[38;2;⟨r⟩;⟨g⟩;⟨b⟩m which should work fine when printing directly to your terminal, since it supports true color.

However when lf displays this preview, it doesn't pass through these terminal sequences directly to the terminal. It actually converts them to tcell.Style objects (e.g. tcell.StyleDefault.Foreground(tcell.NewRGBColor(212, 190, 152))), and then forwards them to the tcell library to perform the actual printing. I'm guessing that for some reason tcell fails to detect that your terminal supports true color, and as a result ends up mapping these colors to a 256-color palette, resulting in the tinted effect.

You can probably test this by using cat bat-output.txt or even printf '\e[38;2;212;190;152mhello world\e[0m\n' directly in the previewer script, and see if it makes any difference depending on whether you have the TERM workarounds or not.

One final thing I'm curious about - is this a regression introduced from r31 to r32, or has this issue always been the case? I don't recall any recent changes that might affect this, but we do bump tcell versions from time to time, which could possibly have triggered this. I don't know if it's too much to ask you to perform a git bisect to help track down the bad commit.

araaha commented 3 months ago

I tracked it down to commit 0c02239

araaha commented 3 months ago

I've further tracked the problem to commit gdamore/tcell/v2@216e30a

araaha commented 3 months ago

As it turns out there was a pull request gdamore/tcell#719 that fixes the problem

joelim-work commented 2 months ago

OK, closing this as a duplicate of #1668 then