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

Handling default/none values in theme config #549

Closed
zwpaper opened this issue Aug 24, 2021 · 3 comments · Fixed by #591
Closed

Handling default/none values in theme config #549

zwpaper opened this issue Aug 24, 2021 · 3 comments · Fixed by #591
Labels

Comments

@zwpaper
Copy link
Member

zwpaper commented Aug 24, 2021

After #452 merged, lsd now supports yaml-based theme configuration.

but a completed configuration file is needed, if any items are missed, the whole theme would fall back to the default one.

we should update the theme struct to Option and allow some items missed.

we should discuss that the missed fields should indicate which one color:

  1. the default foreground color, like no color
  2. the default theme color, falls back the missed fields to the default item.
  3. the default error color, could be used by invalid items
@meain
Copy link
Member

meain commented Aug 26, 2021

I have not been able to take a look at the PR, but being able to skip some would be necessary for it to be useful IMO. My idea is that users should be able to ignore some options and that would use the default color, but at the same time they should be able to specify something like None if they wanna keep it as no color(default terminal foreground).

@zwpaper
Copy link
Member Author

zwpaper commented Aug 26, 2021

Yes, it is necessary! but the theme feature could work functionally without it, so that people could use the feature much earlier if we merge #452 first.

because

  1. we can always add the ability to skip without breaking any currently implemented functions.
  2. supporting one of the options would be an easy job, but it is necessary to support both.
  3. I don't know when did I have time to implement the ability to skip 😂

@meain
Copy link
Member

meain commented Aug 27, 2021

Makes sense ;). Let me go through #452 over the weekend.

@meain meain changed the title theme configuration file should be able to ignore items which use the default foreground color Handling default/none values in theme config Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants