-
Notifications
You must be signed in to change notification settings - Fork 190
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
fix: update to Nerd Fonts 3 code points #264
Conversation
I believe, the proper way of handling this is by adding an option in |
Some obsoleted icons didn't change. nerdfix may help. |
I agree with @Flygrounder, it should be a flag. I'm personally using nf3, but most of the people will be for sure stuck for years in v2. |
just weighing in: according to the Finii from Nerd Font, there should be a "translation" database (under details). I'm not sure if these would be backwards compatible to be honest. Does anyone know? |
@luizkowalski I'd say there are icons (the ones https://www.nerdfonts.com/cheat-sheet) that are not marked as obsolete and are compatible for both versions. |
In my opinion it would always be better to not-use the Material Design Icons (also not the new ones), because they do not have a codepoint-stability guarantee, so with the next update the icon on codepoint X could be exchanged 🙄 It does not happen often, but from time to time if you really compare their releases. Maybe by accident. I think there is a document about the codepoint stability of the various sets: ryanoasis/nerd-fonts#365 (comment). As project I would prefer icons where upstream guarantees that their codepoints will not change. |
@Finii the problem is what you pointed out. MD added now 7k icons, that are probably quite unique and not replaceable. |
👍 And I always check (and try to fix if needed) codepoint stability on any upstream update (see for example the octicons update where Nerd Fonts introduces a codepoint stability where there was none upstream). |
@luizkowalski: that's what I used to convert. I was thinking to wrap translation table into lua, but it seemed wasteful to include table of ~1600 entries. I am curious how implement two sets of icons. I see options:
I am in favour of 3) - easy to drop, easy to generate through translation table from original table in make script. |
thanks, nerdfix did help 🤝 |
It's also worth noting that the new locations of the material design icons ( |
You could also just add a comment on the |
To discover all obsolete symbols in a file:
To fix every obsolete symbol, input a number or search symbols with autocompletion:
|
Thinking about it more, maybe it is ok to merge it. Nerd fonts released version 2.3 earlier this year and deprecated now dropped code points. There are two solutions for people on <=2.2: install newer version (easy), pin commit (easy). Let's just make sure people know what's going on. |
I updated readme and reworded commit so it contains exclamation mark |
I think this is a good approach, one comment I would make is it might be a good idea to include the commit hash in the notice of the last commit that supports Nerd Fonts <= 2.2 for people who want to just pin their commit. |
Totally agree |
Or maybe let's tag (or branch?) that commit so it is more explicit? |
Apologies for interrupting your conversation but I think this PR might be missing some icons. When I've run nerdfix locally I've found the following also need updating
Could just be my version of nerdfont and/or nerdfix though! 😅 |
@tapayne88 you can pull request my master if you wish ;) |
Hopefully this should do it - vlada-dudr#1 🤞 |
@tapayne88, so I merged it into my branch. Now tell me who can move it forward... |
I'll try to review tomorrow. Thanks to everyone involved! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks for the quick fix!
All changes appear to be mdi ->md which is the most straightforward and correct migration.
RE compatibility: providing setup option / both sets is unnecessary complexity.
A nerd-fonts-2
tag along with documentation / readme should be sufficient, pending @gegoune approval.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of nitpicks regarding read me update.
Thanks again!
I have just pushed aforementioned compat tag pointing to current master. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks @vlada-dudr et al for the quick turnaround!
This was really helpful, thanks! |
Nerd Fonts 3 changed code points of some icons, this updates it according to table here
This will break everyone being on Nerd 2, but without it everyone on version 3 is broken also...
fixes #265