refactor color to be under theme so that we can add icon theme

IMPORTANT

this breaks the theme config by moving the origin one to under color:

---

TODO

• [ ] Use cargo fmt
• [ ] Add necessary tests
• [ ] Add changelog entry
• [ ] Update default config/theme in README (if applicable)
• [ ] Update man page at lsd/doc/lsd.md (if applicable)

@zwpaper I think we should probably hash out the details for a breaking change before you put in effort to implementing it. Can we start a thread on this, or continue here. Let's finalize about what exactly the change would be, why it is required and any alternate approaches so as to avoid a breaking changes.

related:

• https://github.com/Peltoche/lsd/issues/343

maybe related:

• https://github.com/Peltoche/lsd/issues/706

@meain this is why I created this draft PR, let's discuss it here directly.

the changes I would like to introduce are that move the current contents of the theme file to become a sub-item of color. the reason is that I want to use one theme file for both color and icons.

we now have color options in the root of the theme file(introduced by me, what a bad choice🤣) for example:

user: 230
group: 187
permission:
  read: dark_green

I would like to make it:

color:
  user: 230
  group: 187
  permission:
    read: dark_green
icon:
  suffix:
    rs: ABCD
    md: 0123

there may be an option to support both

user: 230
group: 187
permission:
  read: dark_green

and

color:
  user: 230
  group: 187
  permission:
    read: dark_green

in the same time, but I think it is not worthy to keep this compatibility code as we can easily notify user and easily update the theme file.

I don't think there is anything necessarily bad about having multiple files, one for colors and one for icons. Besides, I see a few positives for this:

• No breaking changes
• Theme files will not get big (at least the color ones)
• Easier to copy/paste theme(of icon set) from wiki(once we have it)

fine, I did not have a strong opinion on the unique theme file, let's leave it split.

@meain, how about dropping this theme option?

as we discussed in the other issue, the Unicode option is not used in most cases, and the when option should be enough for us.

if a user want to use an Unicode theme, they can create a custom one by this PR's feature.

How about for icons theme, we explicitly call it overrides and keep the theme one with existing functionality. I am not sure if that is the best approach though.

For folks without patched fonts, unicode is an option to get some icons without much hurdle.

---

One other option that I strongly prefer is something different. We don't let users specify a file, instead if a file called colors.yaml is preset in $XDG_CONFIG_HOME/lsd, we load colors from there and if a file called icons.yaml is present, we load icons from there. It feels much simpler. We can support specifying themes in both ways for color for some time with a deprecation message on the old way while people switch. The current way of specifying a file path seems a bit clunky overall.

for the Unicode part, we currently have ONLY 2 icons used, one for folders, and one for files, even more, these two Unicode icons can NOT work in MOST of the systems including the widely used macOS. even that, There is only one issue we received to discuss.

for the default theme file part, I am totally ok with the color.yaml and icon.yaml, the default theme we offered works like those two files, the difference is users can not update the default theme.

BUT offering the ability to specify a file(or path) is a necessary one, as users can save many themes and switch in need. for example, my terminal background color may change between day and night, then I would like to use a different color theme for lsd.

the default theme files could be another enhancement, I have created one to discuss https://github.com/Peltoche/lsd/issues/709

for the default theme file part, I am totally ok with the color.yaml and icon.yaml, the default theme we offered works like those two files, the difference is users can not update the default theme.

The idea is that these files will be how they customize it, not the default theme. I don't think adding the default theme values as files in would be all that useful.

BUT offering the ability to specify a file(or path) is a necessary one, as users can save many themes and switch in need. for example, my terminal background color may change between day and night, then I would like to use a different color theme for lsd.

For that usecase, having the file with a consistent name is a much better approach than changing the path in config.yaml. For example if I can have both colors.yaml and icons.yaml as symlinks to actual themes and just change the symlink paths in a script, or even just copy the original file to the default locations. The alternate way when we have to specify paths in config.yaml would be for them to use some hacky sed script to to use something like yq to change up the value which would be much harder.

the default theme files could be another enhancement, I have created one to discuss https://github.com/Peltoche/lsd/issues/709

Since this will be necessary along with the icons config file change, I think we can track it here itself along with this PR. What do you think?

I don't think adding the default theme values as files in would be all that useful.

I am saying that the file works like users' default theme, but putting the real default theme in it. we have no divergence in this part.

For that usecase, having the file with a consistent name is a much better approach than changing the path in config.yaml.

I don't really think that a symbol link or copying files are better than changing the config.yaml. as we have the config.yaml, it is meant to be the config center, I don't see any problems updating the config.yaml when users want to change the config. a symbol link is more like a workaround if some apps did not have a config file.

so that my point is we can keep both of the features, we reserved the color.yaml and icon.yaml to be the default config, and use the custom one if use specified in the config.yaml. so that

1. did not have any break changes.
2. we satisfied both of the people like you and me.

One more question appeared, should we use the char directly or Hex in config file?

specifying chars is supported now. we have to do something more if we want to support Hex format.

a symbol link is more like a workaround if some apps did not have a config file.

It does not really have to be symlink, it could even be the user replacing the file with a new one there. Here is an example situation. Say the user has a script which runs in the background and change the theme everywhere in the evening. If we have separate files, they can just replace the file. But if the config is just a path, they will have to find some way to edit content within a yaml file which I believe would be harder and might require external tools like yq for example.

so that my point is we can keep both of the features, we reserved the color.yaml and icon.yaml to be the default config, and use the custom one if use specified in the config.yaml.

I was thinking about deprecating the old option as that has been confusing to people. Example: https://github.com/Peltoche/lsd/issues/655 , https://github.com/Peltoche/lsd/issues/566

One more question appeared, should we use the char directly or Hex in config file?

Should the entire escape code work as is? As in, won't they be able to provide something like \u{f016}. If so, that should be enough.

I still believe that specifying the theme in config file is the proper config file usage. no matter symlink or moving files, both seems like the workaround.

as I mentioned above, we can offer the default file, but I don't think we should deprecate the previous one. which also will not make any break changes, I believe that I treat the breaking important more than me.

I still believe that specifying the theme in config file is the proper config file usage. no matter symlink or moving files, both seems like the workaround.

I feel like there is some communication gap here. You can think of colors.yaml and icons.yaml as just two other config files instead of having it be something that you have to link from the "main" config file. For example you can check config files for calcurse(has multiple files like keys, conf), git (has files like attributes, config), mpv (input.conf and mpv.conf), ncmpcpp (bindings, config) etc. These don't link from one another.

as I mentioned above, we can offer the default file, but I don't think we should deprecate the previous one. which also will not make any break changes, I believe that I treat the breaking important more than me.

The reason why I thought we could deprecate the old way was because having multiple ways can possibly be confusing to folks. We have to decide which route we are taking before we go and implement it for icons.

@meain I am still not convinced, but I dropped the custom file option and continued the PR as you insisted.

I check the default icon theme file if no icon theme flag is present, but not handling the color theme file, one reason is that the color theme file has gone public with the previous release, and another is that this PR should limit to icons.

I check the default icon theme file if no icon theme flag is present, but not handling the color theme file, one reason is that the color theme file has gone public with the previous release, and another is that this PR should limit to icons.

Makes sense. We can do the color theme change in a different PR. The idea for color theme file is to support both for a while with only the newer one advertised and depreciate the old option after some time and remove.

Codecov Report

Merging #707 (3f9b016) into master (586e1c9) will decrease coverage by 0.22%. The diff coverage is 91.91%.

@@            Coverage Diff             @@
##           master     #707      +/-   ##
==========================================
- Coverage   87.01%   86.78%   -0.23%     
==========================================
  Files          41       43       +2     
  Lines        4628     4313     -315     
==========================================
- Hits         4027     3743     -284     
+ Misses        601      570      -31     

Impacted Files	Coverage Δ
src/app.rs	75.36% <ø> (ø)
src/core.rs	0.00% <0.00%> (ø)
src/main.rs	26.31% <ø> (ø)
src/theme.rs	78.26% <78.26%> (ø)
src/icon.rs	93.54% <90.36%> (-4.77%)	:arrow_down:
src/color.rs	49.16% <93.33%> (+0.29%)	:arrow_up:
src/display.rs	84.19% <100.00%> (+0.04%)	:arrow_up:
src/flags/icons.rs	97.12% <100.00%> (ø)
src/meta/name.rs	90.41% <100.00%> (+0.03%)	:arrow_up:
src/theme/color.rs	78.18% <100.00%> (ø)
... and 6 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

@meain all works should be ready now. I will leave the prefix/regex to other issues.

Hey @zwpaper , sorry about the delay. I have been pretty busy the past few weeks plus wanted to make a release to help get a deb package if possible. I'll get to this soon.

@meain never mind, it is absolutely ok, look into this any time when you are free

Should we have an option to fallback to existing ones if none was found in the specified theme file? For example if the custom icons.yaml file only has icons for .rs, we can use icons for other filetypes from lsd builtins.

this is how icon theme works now

Some tests are failing if we have a custom theme file in the users system, could you make it so that it load the default themes.

I have a both icon and color theme in my mac, and no error happened, could you point out a case so that I can test it?

I have pushed one commit so that it is easy to review.

after your review is done, I will fix the conflict and force push again.

@meain

this now should be good to go after CICD if there are no other problems.

@meain

thanks for the nice catch! @meain

please take another look.

@meain suggestion committed