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

add "material-nf" icon set, add icons_format config option #1095

Merged
merged 10 commits into from
Feb 27, 2021

Conversation

MaxVerevkin
Copy link
Collaborator

@MaxVerevkin MaxVerevkin commented Feb 24, 2021

With icons_format

icons = "material-nf"
icons_format = " <span size='large' stretch='ultraexpanded' font_family='NotoSans Nerd Font'>{icon}</span> "

image

Without icons_format

icons = "material-nf"

image
Font used

    font pango:JetBrainsMono Nerd Font 10.6

Related issues

TODO

Do not apply icons_format for overridden icons? (Not possible with the current implementation of icons deserialization.)

  • Rename icons_style to icons_format
  • Write documentation
  • Make use of more moon icons (for backlight block).
  • Deprecate old material icon set?

@MaxVerevkin MaxVerevkin changed the title add "material-nf" icin set, add icons_style config option add "material-nf" icon set, add icons_style config option Feb 24, 2021
@MaxVerevkin MaxVerevkin changed the title add "material-nf" icon set, add icons_style config option add "material-nf" icon set, add icons_format config option Feb 24, 2021
@ammgws
Copy link
Collaborator

ammgws commented Feb 25, 2021

Are the Nerd Fonts mappings different than for the official Google material font? I imagine they would be the same if Nerd Fonts is just an aggregator?

@MaxVerevkin
Copy link
Collaborator Author

I checked google's codes and no, they are not the same. That's weird.

@ammgws
Copy link
Collaborator

ammgws commented Feb 25, 2021

Should icons_format be able to be overridden on a per-block basis like with themes?

@MaxVerevkin
Copy link
Collaborator Author

Should icons_format be able to be overridden on a per-block basis like with themes?

I'm not sure. I think that disabling icons_format for icons that were overridden is enough, but needs some more work. Actually, allowing each block to override icons_format it simpler. But current implementation of icons' deserialization is messy and should be refactored anyway. I mean, it uses one specific custom macro that is used just once.

@ammgws
Copy link
Collaborator

ammgws commented Feb 25, 2021

Actually, allowing each block to override icons_format it simpler.

Yeah it might be the way to go. Now too sure but might be useful if for example you wanted to specify some different icon font for the weather block? If it's trivial to implement then I don't really see any downside to allowing users to override it.

. But current implementation of icons' deserialization is messy and should be refactored anyway. I mean, it uses one specific custom macro that is used just once.

Sure

@MaxVerevkin
Copy link
Collaborator Author

Yeah it might be the way to go.

Done.

@MaxVerevkin MaxVerevkin requested a review from ammgws February 26, 2021 09:21
@ammgws
Copy link
Collaborator

ammgws commented Feb 26, 2021

Looks OK to me. As a last step I want to check the differences between the patched material font, the official Google one and the Nerd Fonts one.

Icon Patched Google NerdFonts
bat_charging \u{e1a3} ?? \u{f57d}
etc

@MaxVerevkin
Copy link
Collaborator Author

MaxVerevkin commented Feb 26, 2021

Actually, NerdFonts' README claims that they are using this material icons, not Google's - https://github.com/Templarian/MaterialDesign

Google's font has only one charging icon - battery_charging_full e1a3

I haven't found any icons that would "work" in any two of those fonts at the same time yet.

@ammgws
Copy link
Collaborator

ammgws commented Feb 26, 2021

@MaxVerevkin
Copy link
Collaborator Author

possibly patched but codepoints the same?

From what I can tell, no, codepoints are different.

@ammgws
Copy link
Collaborator

ammgws commented Feb 27, 2021

Looks like it. Let's deal with it separately.

Thanks for this!

@ammgws ammgws merged commit 1b3eabc into greshake:master Feb 27, 2021
@MaxVerevkin MaxVerevkin deleted the material-icons branch March 14, 2021 09:21
@ammgws ammgws mentioned this pull request Mar 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants