sayanarijit / xplr

A hackable, minimal, fast TUI file explorer
https://xplr.dev
MIT License
4.08k stars 74 forks source link

Setting foreground directory colors via style is broken? #620

Closed reissbaker closed 1 year ago

reissbaker commented 1 year ago

Hello! I recently upgraded to v0.21.1 and noticed that the foreground directory colors looked a little off on my machine. Figuring that these were just the default color settings, I tried overwriting the foreground colors in two ways:

xplr.config.node_types.directory.style = {
  fg = "White"
}

And, since that didn't work, I also tried just setting the attribute alone:

xplr.config.node_types.directory.style.fg = "White"

But still no dice. FWIW, setting the background color this way works; for example:

xplr.config.node_types.directory.style.bg = "White"

Works! So I'm guessing this is just a bug in merging the default directory color with the init.lua directory color?

sayanarijit commented 1 year ago

Can you try unset LS_COLORS? Or set LS_COLORS accordingly using something like https://github.com/sharkdp/vivid...

reissbaker commented 1 year ago

Unfortunately, running LS_COLORS="" LSCOLORS="" xplr still seems to ignore the xplr.config.node_types.directory.style.fg = "White" setting.

reissbaker commented 1 year ago

(I'm running this in zsh on Ubuntu, FWIW, but I figured I'd try both LS_COLORS and the macOS-/FreeBSD-style LSCOLORS just in case you were parsing both.)

reissbaker commented 1 year ago

That being said — I think the bug actually mostly occurs if you:

  1. Have no LS_COLORS set, and
  2. Try to set the foreground color manually

When I manually set LS_COLORS, xplr respects them; but when there are none, it a) doesn't, and b) doesn't let me actually set the directory foreground colors.

I feel like my expectation would be the following priority:

  1. Highest priority is colors set in init.lua
  2. Second highest is colors from LS_COLORS

I could also understand (although would find vaguely surprising):

  1. Highest priority is LS_COLORS
  2. If there are no LS_COLORS set, then whatever is in init.lua

But instead it seems like:

  1. Colors in LS_COLORS
  2. Some default colors?
  3. (It is impossible to set the foreground colors of directories from init.lua)

Which seems like a bug?

sayanarijit commented 1 year ago

Agree with Highest priority is colors set in init.lua.

Initially, I thought of giving LS_COLORS more priority because it's easier to change than init.lua, thus easy to switch between different themes.

But now I learned that applications by convention default to a set LS_COLORS theme even if it's not set or is set to an empty string, and it is expected to be overridden by the application specific theming.

So yes, making LS_COLORS low priority makes more sense.

sayanarijit commented 1 year ago

Pls test https://github.com/sayanarijit/xplr/pull/622 and let me know how it works...

sayanarijit commented 1 year ago

Update: With LS_COLOR having less priority, I think now it makes sense to unset the defaults.

reissbaker commented 1 year ago

Such a fast fix! Tested and it works on my machine 😁