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

Icon theme with overrides from config #780

Closed
ElPapiMango opened this issue Dec 11, 2022 · 5 comments · Fixed by #792
Closed

Icon theme with overrides from config #780

ElPapiMango opened this issue Dec 11, 2022 · 5 comments · Fixed by #792
Labels
kind/bug Something isn't working

Comments

@ElPapiMango
Copy link

  • os: debian
  • lsd --version: lsd 0.23.1
  • echo $TERM: Windows Terminal
  • echo $LS_COLORS:

Current behavior

Created icons.yaml in ~/config/lsd folder based on example in documentation. Docs says

"The final set of icons used will be a combination of what is shipped with in lsd with overrides from config applied on top of it".

However this does not work for me. Icons from what is shipped is gone anď only this in icons.yaml is used.

Using icon theme. Config.yaml and icons.yaml has a default icon.

image

Removing icon theme and now config.yaml and icons.yaml have the icon that is shipped.

image

@zwpaper
Copy link
Member

zwpaper commented Dec 13, 2022

icon theme is currently on master and not yet released, please try to build master or wait until we release 0.24.0

@ElPapiMango
Copy link
Author

Using master though and rebuilt it today and same issue.

@sudame
Copy link
Contributor

sudame commented Jan 12, 2023

I found that if we set extension field in icons.yaml, the default theme for extension is unexpectedly ignored. The same thing happens when we set name field and filetype field.

Speaking on a code basis, I believe the following is the cause of this bug.
https://github.com/Peltoche/lsd/blob/da61909835b281d7f7950ec3f763cd077b5b8f02/src/theme.rs#L78-L86

The return value typed D has Default trait so Serde (Rust's (de)serialize library) will use default field if the field is not set. However in this case a user sets the field so the default field must be ignored.

example

When you set icons.yaml like:

extension:
    rs: 🦀 

lsd constructs IconTheme struct like:

name:
    a.out: \u{f489}
    api: \u{f98c}
    # and the other default values...
extension:
    rs: 🦀
    # ↑ only!
filetype:
    dir: \u{f115}
    file: \u{f016}
    # and the other default entries...

but we want like this:

name:
    a.out: \u{f489}
    api: \u{f98c}
    # and the other default entries...
extension:
    rs: 🦀
    1: \u{f02d}
    2: \u{f02d}
    # and default entries other than those set in the configuration file...
filetype:
    dir: \u{f115}
    file: \u{f016}
    # and the other default values...

Edit

I found that filetype can marge icons.yaml with the default values because IconTheme.filetype is a struct with Default tarit (the others are HashMap<String, String>).

@meain
Copy link
Member

meain commented Jan 14, 2023

@sudame would you like to take a shot at creating a fix for this?

@meain meain mentioned this issue Jan 14, 2023
6 tasks
@sudame
Copy link
Contributor

sudame commented Jan 14, 2023

@meain OK, I'll try this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants