yaml based theme support

[zwpaper]

fix #92
related: #365

[codecov-io]

Codecov Report

Merging #452 (f701655) into master (b6bd1d3) will increase coverage by 1.45%.
The diff coverage is 81.22%.

@@            Coverage Diff             @@
##           master     #452      +/-   ##
==========================================
+ Coverage   80.71%   82.16%   +1.45%     
==========================================
  Files          35       36       +1     
  Lines        3449     3494      +45     
==========================================
+ Hits         2784     2871      +87     
+ Misses        665      623      -42     

Impacted Files	Coverage Δ
src/core.rs	0.00% <0.00%> (ø)
src/flags.rs	87.50% <ø> (ø)
src/main.rs	0.00% <0.00%> (ø)
src/meta/mod.rs	20.90% <33.33%> (+2.98%)	⬆️
src/color/theme.rs	34.90% <34.90%> (ø)
src/color.rs	70.86% <70.23%> (-8.14%)	⬇️
src/config_file.rs	76.66% <76.40%> (+58.85%)	⬆️
src/display.rs	24.24% <80.00%> (-1.25%)	⬇️
src/flags/size.rs	94.33% <89.28%> (+5.45%)	⬆️
src/flags/color.rs	91.24% <90.10%> (-0.33%)	⬇️
... and 26 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b6bd1d3...f701655. Read the comment docs.

[zwpaper]

@meain there is a theme config no-lscolors maybe we need to discuss.

previously, we use LSCOLORS by default, so that there is a configuration no-lscolors, but after this theme implementation, no-lscolors seems has not effect, should we change it to lscolors and read LSCOLORS instead of theme file.
but doing this, may break some compatibility if people used --no-lscolors before.

[meain]

Hey @zwpaper , we had never exposed --no-lscolors to the user. It was an internal only thing.

Even if we implement themes via config file, we should still have LS_COLORS used by default as that will work better with user's existing terminal colors. All they would need to override this would be to specify a theme in the config file. I believe most people will expect LS_COLORS to work as that is a more general standard across multiple tools.

Also, I think we should let the user configure the values of non-name blocks via the config file but still let them use LS_COLORS for the name block. There are projects like trapd00r/LS_COLORS which has great support for coloring files based on their extension etc. I am not exactly sure as how to approach this yet though.

[meain]

Two more things:

• Color names would be better as lowercase values
• Need to support something like default for a color value which would not apply any color (uses terminal foreground color)

[zwpaper]

waiting on this PR ogham/rust-ansi-term#71

[zwpaper]

As we were blocked by the ansi-term PR, I will later try a new crate https://github.com/crossterm-rs/crossterm , it seems more active.

I will do some tests and benchmarks for the change later

[zwpaper]

new PR created crossterm-rs/crossterm#576

[zwpaper]

status update, I have reimplemented the theme support based on crossterm, it could be built and basically work as expected with my latest commit.

I will then work on the tests and make sure not to break the original functions.

[codecov-commenter]

Codecov Report

Merging #452 (bb6e7f9) into master (62b1eec) will decrease coverage by 1.71%.
The diff coverage is 70.02%.

@@            Coverage Diff             @@
##           master     #452      +/-   ##
==========================================
- Coverage   88.44%   86.72%   -1.72%     
==========================================
  Files          36       37       +1     
  Lines        3540     3790     +250     
==========================================
+ Hits         3131     3287     +156     
- Misses        409      503      +94     

Impacted Files	Coverage Δ
src/core.rs	0.00% <0.00%> (ø)
src/flags.rs	100.00% <ø> (ø)
src/flags/layout.rs	100.00% <ø> (ø)
src/main.rs	7.69% <ø> (ø)
src/config_file.rs	71.59% <36.84%> (+2.36%)	⬆️
src/color.rs	56.28% <52.55%> (-22.36%)	⬇️
src/color/theme.rs	55.00% <55.00%> (ø)
src/flags/color.rs	91.91% <89.83%> (-1.68%)	⬇️
src/display.rs	75.70% <100.00%> (+0.29%)	⬆️
src/flags/size.rs	95.65% <100.00%> (+0.33%)	⬆️
... and 9 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 62b1eec...bb6e7f9. Read the comment docs.

[TheTrio]

Any updates about this PR? Can't wait to have custom theme support.

Thanks!

[zwpaper]

pin bitflags dependency to 1.2.1 due to clap-rs/clap#2691

[zwpaper]

hi @meain, I have done this implementation, could we leave the code coverage for later improvement.

and for the comment:

1. color value would ignore the cases so that both lowercase and UPPERCASE would work, I have added the description to readme.
2. new issue Handling default/none values in theme config #549 is created for the default part, we may discuss it there and let the theme support release first

[zwpaper]

finally, this one-third year PR is coming true 🤩

[meain]

Hey, just a have few changes.

Also, for some reason the colors for the filenames(lscolors) look different compared to how it was before when I have no lscolors set. Let me see if I can figure out what is going on.

[zwpaper]

new issue created for the NoLscolor dead code #552

[meain]

Hey, I was looking more into the original color issue that I was talking about. The color values being outputted is different. From what I can tell, old one was using default colors and the new one is using bright colors. If the shell is set up with different normal and bright colors, this will cause issues.

cargo run -- -ld --color always ci > filename && vim filename

new:

�[38;5;12m�[1md�[0m�[38;5;10mr�[39m�[38;5;11mw�[39m�[38;5;9mx�[39m�[38;5;10mr�[39m�[38;5;245m-�[39m�[38;5;9mx�[39m�[38;5;10mr�[39m�[38;5;245m-�[39m�[38;5;9mx�[39m �[38;5;229m160�[39m�[38;5;229mB�[39m �[38;5;36m3 months ago�[39m �[38;5;12m�[1mci�[0m

old:

�[1;34md�[0m�[32mr�[33mw�[31mx�[32mr�[38;5;245m-�[31mx�[32mr�[38;5;245m-�[31mx�[0m �[38;5;229m160B�[0m �[38;5;36m3 months ago�[0m �[1;34mci�[0m

Something like this seems to fix the color issues visually, but the output is still different between the two:
meain@f3ff442 (edit: the difference I think is just because crossterm uses 256 colors)

Dropping this here for reference: https://misc.flogisoft.com/bash/tip_colors_and_formatting

[meain]

Also, lscolors supports crossterm. We should be using that instead of converting it internally btw. https://github.com/sharkdp/lscolors/blob/bc95124f23d2f1c14616e6a97f5ae9e6ab786697/Cargo.toml#L22

[zwpaper]

@meain I have changed to dark color, for the consistency with the older version.

for the lscolors supported crossterm part, there is not yet a released version supporting the crossterm, I created an issue for a newer version sharkdp/lscolors#34.

I also noticed that the lscolors map its red to the red both ansi_term and crossterm, maybe we should pick up an ansi value instead of using the color name later.

but the choice of default colors would be a new problem.

[zwpaper]

btw, I think we could make theme happen and do not have to wait for the sharkdp/lscolors#34, we can change to the lscolors implementation any time later.

[meain]

Looks great, I think we are good unless you have something else to add/tweak.

[zwpaper]

hi @meain, I have done the jobs in my mind currently, let's make this real now! we can add the others in other PRs.

[meain]

Awesome, thanks a lot @zwpaper for working on this.