Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Themes #92

Closed
erusev opened this issue Dec 27, 2018 · 24 comments · Fixed by #452
Closed

Themes #92

erusev opened this issue Dec 27, 2018 · 24 comments · Fixed by #452
Assignees
Labels
kind/feature New feature request

Comments

@erusev
Copy link

erusev commented Dec 27, 2018

It seems like the colors that lsd uses are hard coded. It'd be awesome if it supported custom colors.

I'd love to develop a theme that uses colors for file names and shades of gray for everything else - lighter shade for more recent dates and larger file sizes.

@erusev erusev closed this as completed Dec 27, 2018
@erusev erusev reopened this Dec 27, 2018
@erusev erusev changed the title theme Themes Dec 27, 2018
@Peltoche
Copy link
Collaborator

Hi @erusev ,

There is currently a PR about an integration with the LS_COLOR variable (#84), it should allow to customize some of the colors. If you need a deeper color integration, for the sized for example, I think it would be possible to create a theme, the colors.rs module can be easily customized for it. The only thing is that for now the themes are hardcoded and if everyone add his own, it will be a real mess.

A possible solution would be to create a new flags allowing to load a file describing the color theme. Something like:

lsd --theme-file=~/.my-custom-theme.json

and inside ~/.my-custom-theme.json:

{
 "file": 230, // Cornsilk1
 "dir": 33, // DodgerBlue1
 ...
}

What do you think of it?

@erusev
Copy link
Author

erusev commented Dec 27, 2018

Yes, making it possible to load the theme from a separate file would be ideal - it could be a flag that sets it or a config variable as in https://github.com/sindresorhus/pure (under the options section).

@kmoschcau
Copy link
Contributor

Hi, I am interested in working on this when I find the time, I have some questions though:
Do we want to use JSON or should we maybe use something more human friendly like YAML?
And if we add a config file like that, should we make it possible to put more than just the theme in there?
And finally, if we add config files, I would like to support the XDG Base Directory Specification. What do you guys think?

@Peltoche
Copy link
Collaborator

Peltoche commented Nov 12, 2019

Hi @kmoschcau , thanks for the proposition!

Do we want to use JSON or should we maybe use something more human friendly like YAML?

Personally I prefer json but I guess that YAML is more adequate for human redable configurations. If it's possible to handle the two it would be perfect but it's not a priority.

And if we add a config file like that, should we make it possible to put more than just the theme in there?

Absolutely! The goal would be to accept any flags as a configuration.

And finally, if we add config files, I would like to support the XDG Base Directory Specification. What do you guys think?

I guess it would be a good idea. In any cases, it would be great if we can override the default (or XDG) path by one specified as an argument.

Don't hesitate, if you have more questions or if you need reviews

@kmoschcau
Copy link
Contributor

I am not sure about arguments to set the paths, but the specification lists some environment variables to set those paths. But the rest sounds good to me, I hope I can find the free time to work on this soon.

@kmoschcau
Copy link
Contributor

One thing I just found out about recently: YAML is actually a superset of JSON, so any valid JSON is also valid YAML. So a good YAML parser should support both.

@vks
Copy link

vks commented Nov 12, 2019

What about TOML? It serves a similar purpose as YAML, and it is quite a bit simpler.

@kmoschcau
Copy link
Contributor

That depends a bit on what kind of config values one wants to store. TOML could actually be better for this case (because simple), but I have not really seen it outside of the Rust ecosystem yet. Might confuse some users, but also might not because it is so simple.

@kmoschcau
Copy link
Contributor

I just had a look at the TOML specification and to be honest, if it gets to more complicated stuff I find YAML to be easier to comprehend as a user. I will have a look at a first implementation with YAML.

@kakulukia
Copy link

Bildschirmfoto 2019-11-22 um 22 00 59

This is my config for colorls and while the multiple colors for different file types is cool, 10 different colors is a lil much. Im all in for themes!

@kmoschcau
Copy link
Contributor

kmoschcau commented Nov 30, 2019

@Peltoche I have a question concerning code organization. Since the Flags::from_matches function is already fairly large, I started to implement constructor functions for every single flag enum directly. I also wanted to add test methods for those constructor functions. However this will really blow up the size of the flags.rs file. So I would suggest moving those flag enums into their own respective files and put all of that into its own module dir. Since this is a pretty substantial change, I wanted to have your opinion on it first.

@Peltoche
Copy link
Collaborator

Hi @kmoschcau, I think this split would make total sens. Moving the flag parsing next to the meta options would make it easier to read as we can see all the flag lifecycle in one place.

@kmoschcau
Copy link
Contributor

kmoschcau commented Dec 12, 2019

Some things also came to mind: We probably need some additional flags or just simply --no-... versions of flags to disable behavior that is enabled by the config.

Also I think the --oneline flag name is misleading. It should be something along the lines of --one-per-line or something like that.

And another thing I came across that is really confusing: Why is the --group-dirs option (and some others as well) set up to be able to be specified multiple times, but it only ever uses the last value given?

@kmoschcau
Copy link
Contributor

Just to give an update. I am so far done with implementing on my side, I just have some catching up to do with master. I hope I don't miss anything. :D

@swedneck
Copy link

swedneck commented Jun 2, 2020

any updates on this? lsd looks lovely but not using $LS_COLORS is a rather big turnoff for me

@kmoschcau
Copy link
Contributor

I am still waiting on my configuration file merge request to be merged. Once that happens, I would start with adding theme configuration things.

@kakulukia
Copy link

@Peltoche - please merge :)

@zwpaper
Copy link
Member

zwpaper commented Oct 19, 2020

hi @kmoschcau , may I ask how is the progress of theme configuration?

maybe we can split the work and make it happen faster together

@zwpaper
Copy link
Member

zwpaper commented Oct 24, 2020

as this is requested from time to time, I am trying to start my work for lsd theme, If anyone has started, please let me know!

@zwpaper zwpaper self-assigned this Oct 24, 2020
@kmoschcau
Copy link
Contributor

This should certainly be easier now, since you can now save if in the config file. I'm not sure whether it would be beneficial to be able to specify an entire theme via CLI options.

@zwpaper
Copy link
Member

zwpaper commented Oct 25, 2020

Hi @kmoschcau , thanks for the work on the config file.

I don't think to specify an entire theme via CLI options would be a great idea, this would make the options too complicated.

I will implement one option(and one in the config file) to specify the path of the theme, as a standalone theme file would be easier to switch between themes.

@kmoschcau
Copy link
Contributor

There is some merit to putting themes into their own files. But have you also considered putting them in the config file with the key being their name?

@zwpaper
Copy link
Member

zwpaper commented Oct 25, 2020

putting themes in a single config file make it simpler, that is good!

but I preferred standalone file because:

  1. changing themes always replaces the whole theme, and not much chance changes some color inside it.
  2. standalone file makes it easier for sharing

the theme name as the key may also be a good choice, but in my personal experience, I don't like the dynamic yaml key much...

@kmoschcau
Copy link
Contributor

Well there wouldn't really be much dynamic about. The way I would make it is to have a root level key themes, which has an array of theme name keys, which then hold the color specifications.

But yes you could also just replace that mechanic by pulling out the color specifications into their own files. If you do that, it is worth thinking about putting those into their own subdir.

zwpaper added a commit to zwpaper/lsd that referenced this issue Nov 29, 2020
fix lsd-rs#92

Signed-off-by: zwPapEr <zw.paper@gmail.com>
zwpaper added a commit to zwpaper/lsd that referenced this issue Nov 29, 2020
fix lsd-rs#92

Signed-off-by: zwPapEr <zw.paper@gmail.com>
zwpaper added a commit to zwpaper/lsd that referenced this issue Nov 29, 2020
fix lsd-rs#92

Signed-off-by: zwPapEr <zw.paper@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants