Support for bytes separated by thousand with comma

[arkadiuszbielewicz]

Solution for task #533.
Used num-format library as it's really simple to use and seems to be performant. It also allows further extension to support OS locale and grouping not only by thousands but also in Indian style. Used Locale.en as it's most obvious.

---

TODO

• Use cargo fmt
• Add necessary tests
• Add changelog entry

[codecov-commenter]

Codecov Report

Merging #569 (6f1b202) into master (676d5ab) will decrease coverage by 0.52%.
The diff coverage is 89.65%.

@@            Coverage Diff             @@
##           master     #569      +/-   ##
==========================================
- Coverage   86.40%   85.88%   -0.53%     
==========================================
  Files          37       37              
  Lines        3848     3869      +21     
==========================================
- Hits         3325     3323       -2     
- Misses        523      546      +23     

Impacted Files	Coverage Δ
src/app.rs	72.58% <ø> (+3.92%)	⬆️
src/meta/size.rs	94.35% <86.95%> (+0.10%)	⬆️
src/flags/size.rs	94.66% <100.00%> (-0.99%)	⬇️
src/color.rs	51.19% <0.00%> (-5.36%)	⬇️
src/flags/recursion.rs	88.13% <0.00%> (-3.39%)	⬇️
src/config_file.rs	69.31% <0.00%> (-2.28%)	⬇️
src/meta/name.rs	92.44% <0.00%> (-1.34%)	⬇️
src/color/theme.rs	53.75% <0.00%> (-1.25%)	⬇️
src/flags/blocks.rs	93.01% <0.00%> (-0.88%)	⬇️
... 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 676d5ab...6f1b202. Read the comment docs.

[meain]

Hey, thanks for working on this.
I remember you mentioning about the cland dependency for picking system locale, but without that I'm not sue how useful this will be. Any thought @zwpaper ?

I found a related issue in exa for this ogham/exa#554.

[zwpaper]

the issue in exa and lsd shows there seems to be someone who cares about showing the separator, but it may really be strange for the people not using , as thousand separators.

it is also not worth adding the clang dependency for such a minor feature...

I have checked the num-format lib, and found that there is no need for clang for the local feature, the clang is only necessary for auto-detecting the format, how about using the local only, and leave the style to cmd args, configuration file or shell env. @meain

[meain]

locale is something that should be picked up from the users env as they will have configured that in the OS. Don't think we should ask the user to configure locale setting again specifically for lsd. Maybe we could implement the locale detection of linux and macos alone and skip that in windows. In Windows we can just use 1000 based separator until we find a pure rust lib. There is an alternate crate as well if we are going that route: https://docs.rs/locale/0.2.2/locale/

[arkadiuszbielewicz]

Exactly as @meain mentioned, my idea was to use only part which doesn't require Clang (it's only used for SystemLocale feature) and with some other code, detect system locale. Should I do it within this PR?

[meain]

What do you have in mind to get system locale? I think it is better gone in as a single PR just to validate the idea. @arkadiuszbielewicz

[arkadiuszbielewicz]

Hi, I've checked available options and finally I've decided to use num_format with with-system-locale feature, but only for Unix family. In case of Windows it's disabled with fallback to Locale.en.

[zwpaper]

Hi @arkadiuszbielewicz, could you please fix the CI error, and rebase onto the master, also squash your commits for a git clean history.