Color output based on LSCOLORS

[meain]

Uses https://github.com/sharkdp/lscolors to color the output.

Fixes #28

• Should other colors be changed based on LSCOLOR? I think exa does something like that.
• Option to override colors?

[Peltoche]

Hi @meain,

Brace yourself because it's not an easy one. I was thinking about this issue since a moment and it would be super cool if we success to contains all the lscolor logic inside the src/color.rs module. It will not be easy. We can possibly do it if we can make a contribution to the lscolors with a new method Colorls::style_for_type(kind: FileType) (or something like that) where FileType would be an enum with all the file types (directory/symlink/etc). If we have this we can use the color::Colors::colorize() function whithout changing anything because we would be able to create a map matching our color::Elem with the lscolors::FileType and returning the correct color.

What do you think of it?

[meain]

FileType in lscolors right now is basically just a sting.

How about something like style_for_path_with_metadata whose signature looks like

    pub fn style_for_path_with_metadata<P: AsRef<Path>>(
        &self,
        path: P,
        metadata: Option<&std::fs::Metadata>,
    ) -> Option<&Style> {}

but instead of metadata we pass in the type which is an Option<enum> of (directory/symlink,broken-sylink/../../) which is available lsd/color.rs. We would just be skipping the check in the function and using the data available in lsd/color.rs

[Peltoche]

Seems cool. I think you can simplify the function a little more by replacing the path: Path by path: String.

[Skarlett]

Even after a successful build, There's an unhandled exception in this merge.

 ~ > RUST_BACKTRACE=1 ./lsd .                                                                                                                                                      
thread 'main' panicked at 'failed to get the group name', libcore/option.rs:1008:5
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::print
             at libstd/sys_common/backtrace.rs:71
             at libstd/sys_common/backtrace.rs:59
   2: std::panicking::default_hook::{{closure}}
             at libstd/panicking.rs:211
   3: std::panicking::default_hook
             at libstd/panicking.rs:227
   4: std::panicking::rust_panic_with_hook
             at libstd/panicking.rs:476
   5: std::panicking::continue_panic_fmt
             at libstd/panicking.rs:390
   6: rust_begin_unwind
             at libstd/panicking.rs:325
   7: core::panicking::panic_fmt
             at libcore/panicking.rs:77
   8: core::option::expect_failed
             at libcore/option.rs:1008
   9: <core::option::Option<T>>::expect
             at libcore/option.rs:322
  10: <lsd::meta::owner::Owner as core::convert::From<&'a std::fs::Metadata>>::from
             at src/meta/owner.rs:20
  11: lsd::meta::Meta::from_path
             at src/meta/mod.rs:63
  12: lsd::core::Core::list_folder_content
             at src/core.rs:157
  13: lsd::core::Core::run_inner
             at src/core.rs:82
  14: lsd::core::Core::run
             at src/core.rs:55
  15: lsd::main
             at src/main.rs:44
  16: std::rt::lang_start::{{closure}}
             at libstd/rt.rs:74
  17: std::panicking::try::do_call
             at libstd/rt.rs:59
             at libstd/panicking.rs:310
  18: __rust_maybe_catch_panic
             at libpanic_unwind/lib.rs:102
  19: std::rt::lang_start_internal
             at libstd/panicking.rs:289
             at libstd/panic.rs:392
             at libstd/rt.rs:58
  20: std::rt::lang_start
             at libstd/rt.rs:74
  21: main
  22: __libc_start_main
  23: _start
EXIT CODE: 101
 ~ > 

OS: Arch Linux
Arch: x86_64 Intel
Shell: zsh

[meain]

@Skarlett I'm guessing that is related to the issue #87.

[meain]

@Peltoche I was trying to get the colouring working with the new lscolors using style_for_indicator. One issue that I got into while doing that is that I have a highly customized LSCOLRS variable as in it has specific colors for each file type (different colors for different file extentions) but that data is not available in the Elem nor can it be passed when using the Indicator. I feel like we might have to pass the whole filename through to colors.rs.

[Peltoche]

Hi @Skarlett , this bug should be fixed into the recent after the 0.11.0. If you keep having the bug with a later version can you create an issue please?

[Peltoche]

Hi @meain. It seems that the extension feature will be difficult to implement for now because for what I have seen the lscolor package doesn't support it neither.

What do you think about implementing a more basic implementation (without the extensions) for now and then improving it later?

[sharkdp]

Hi @meain. It seems that the extension feature will be difficult to implement for now because for what I have seen the lscolor package doesn't support it neither.

If you mean the lscolor crate: yes, it does support colorization by file extension.

[Skarlett]

Hi @Skarlett , this bug should be fixed into the recent after the 0.11.0. If you keep having the bug with a later version can you create an issue please?

Yessir, you can count on that.

[Peltoche]

If you mean the lscolor crate: yes, it does support colorization by file extension.

Arf, my bad... If the filename is required in order to make a correct implementation let's go for it then.

[meain]

@Peltoche I am not sure about the approach that I want to use for this. This is what I have come up for now.

We will retain the colorize function to do what it is currently doing. We can use Indicator based coloring for these(or load and store the associated color in the begining). We will also have another function, maybe something like colorize_file which will be specific for styling file names.

There will be some recomputation as to the type of the file but this seems like a simple way to approach this for now. Do you have any other idea as to how to approach this?

Also I think we should restrict the other colors to 1 to 15. This will help to have the colors based on the colorscheme

[meain]

This is the first draft, see if it looks good. @Peltoche

[Peltoche]

It look cool, I think that a fallback on Self::colorize would be great.

[meain]

@Peltoche The build on travis seems to fails as it cannot compile lscolors on 1.30 as it specifies Rust 2018
https://github.com/sharkdp/lscolors/blob/c9509642ef916e7cced3d60ed50a5dfbc7a962d5/Cargo.toml#L17

[Peltoche]

@meain I guess we are forced to drop the 1.30 and align ourselve with the rust 2018 edition...

I make a PR which should solve it.

[Peltoche]

It's almost good! There is just some that we can simplify a little.

[Peltoche]

Do we merge it?

[meain]

Yeah, I guess this is good to merge now.