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

FontAwesome: Reorder some glyphs #1596

Merged
merged 10 commits into from
Apr 10, 2024
Merged

FontAwesome: Reorder some glyphs #1596

merged 10 commits into from
Apr 10, 2024

Conversation

Finii
Copy link
Collaborator

@Finii Finii commented Apr 8, 2024

Description

Fixes: #1563 (comment)

Hi, I detected that the glyph nf-fa-circle_thin at \uf1db () now is presented as a filled circle but before it was a thin ring. This exactly glyph is used in powerlevel10k to identify sr.ht git remote, I'm adding it to lazygit with the same purpose, that's how I found the problem.

From @hasecilu

More details in the comments.

Requirements / Checklist

What does this Pull Request (PR) do?

How should this be manually tested?

Any background context you can provide?

What are the relevant tickets (if any)?

Fixes: #1588
Fixes: #1593

Screenshots (if appropriate or helpful)

@Finii Finii added the Bug fix label Apr 8, 2024
@Finii Finii added this to the v3.2.1 milestone Apr 8, 2024
@Finii
Copy link
Collaborator Author

Finii commented Apr 8, 2024

Thanks for reporting!

I guess the reason is that FA comes now with 3 sets of icons and we select one of the sets for each codepoint (script remix), and the super smart algorithm is not clever enough ;-)

Let me quickly check them all (again)

image

image

image

code observation result
F000 not filled This is from solid, there is no filled glass anymore
F009-F00B look inverse that is the new look
F05C - F95D check-marks became arrows Error: These should be from regular, where they are not the main codepoints
F0E6 talk bubble became plane Error: Icon should have been taken from regular
F10A - F10B now filled Error: F10A -> F3FA and F10B -> F3CD
F14D inverse? is already from solid, there is no inverse
F16A moved to F167 on dropping one YT icon Error: swap F167 and F16A
F172 inverse bitbucket missing? They dropped it, now there is the same icon twice in upstream
F1CC inverse? Thats the only jsfiddle they have
F1DB now filled Error: Take from regular instead
F20C "mp" dropped? dropped upstream
F219 not a diamond / gem anymore Error: use F3A5 from regular
F230 facebook square missing? has been dropped
F254 filled hourglass now unfilled there is no filled anymore
F262 what became of the old logo tripadvisor dropped
F27C " upstream substituted that
F289 " "
F29E now inverse? Thats the new audio description
F2AC check all snapchat icons dropped upstream
F2B4 now not filled new font awesome logo already from solid
F2D8 inverse upstream change
F2D9 - F2DA icon change? upstream change

Action plan

  • F05C - F95D from regular
  • F0E6 from regular
  • F10A -> F3FA and F10B -> F3CD
  • swap F167 and F16A
  • F1DB from regular
  • F219 -> F3A5

@Finii Finii mentioned this pull request Apr 8, 2024
2 tasks
[why]
The open variants of circle_xmark and circle_check are missing.

[how]
Rename the filled versions to their old (i.e. FA 4.2) names
(remove_sign and ok_sign) and let the open (regular) variants
find their place automatically.

Because we now have two more icons that would move all codepoints we can
either drop the icons that occupied the circle_xmark and circle_check
codepoints before, or we move the to the end (unpatched yet) region.

As these are just some more arrows, we drop them.

Dropped:
F30B F05C solid/right-long.svg
F30C F05D solid/up-long.svg

New:
F05C F05C regular/circle-xmark.svg
F05D F05D regular/circle-check.svg

Renamed:
RENAME circle_xmark to remove_sign (F057)
RENAME circle_check to ok_sign (F058)

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why]
When creating the renaming table the wrong number has been entered
(twice the same).

[how]
Correct the renaming thereby making the renamed icon visible.
The icon that has been 'kicked out' is moved to the end of the patched
block.

New:
F0E6 F0E6 regular/comments.svg

Moved:
F374 EFC2 brands/avianex.svg (was previously on F0E6)

Renamed:
RENAME comments_o to comment_o (F0E5)

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
youtube and cloudsmith should change their codepoints, because youtube
occupies a 'lesser' youtube codepoint and yields a 'better' codepoint to
cloudsmith.

Better means the icon looks more like in FA 4.

Also swap the devices icons that in 4 showed a screen and now it is
filled.

[how]
The mechanics is a bit ... ugly.

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
@Finii
Copy link
Collaborator Author

Finii commented Apr 8, 2024

target was shall comment commit
F05C F30B solid/right-long.svg F05C regular/circle-xmark.svg 1
F05D F30C solid/up-long.svg F05D regular/circle-check.svg 1
F0E6 F374 brands/avianex.svg F0E6 regular/comments.svg 2
F10A F10A solid/tablet-button.svg F3FA solid/tablet-screen-button.svg use different icon 5
F10B F10B solid/mobile-button.svg F3CD solid/mobile-screen-button.svg use different icon 5
F167 F167 brands/youtube.svg F384 brands/cloudsmith.svg swapped 3
F16A F384 brands/cloudsmith.svg F167 brands/youtube.svg swapped 3
F1DB F1DB solid/circle.svg F1DB regular/circle.svg other set 4
F219 F219 solid/diamond.svg F3A5 regular/gem.svg 3

[why]
F1DB had an open circle, called circle-thin.
That must now come from the regular set.
Unfortunaly circle_o at F10C already is also the same regular set icon.
And we try to prefer the solid set if a regular set has been used
already.

[how]
Add one specific exception to the rule :-(

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why]
Previously we had icons with an open screen and a button, now it is
filled.

[how]
Swap the icons with the open icons at completely different codepoints.

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why]
We do not want to break the Fira progress icons, but on the other hand
these are so few icons that we can not keep the whole 0x0100 block
unoccupied.
So we just leave a small gap in the FontAwesome codepoints.

[how]
Be careful, the current code moves the icons 'to the back', which will
be different if we ever update FontAwesome. The code will break
(reassign a lot codepoints) then.

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Don't forget to run against the v3.1.1 glyphnames.json.

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why]
The glyphs can look a bit small(er) when used unscaled, for example in a
Nerd Font Propo font. Note that some icons indeed got smaller, but in
general the generated font is a bit smaller than before.

Related: #1588

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
@Finii Finii marked this pull request as ready for review April 8, 2024 16:18
Copy link
Contributor

@hasecilu hasecilu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I regenerated the Symbols font with these changes and seems to be fixed:
image

@Finii
Copy link
Collaborator Author

Finii commented Apr 9, 2024

@hasecilu Thanks for having a look! 💚

This release seems to have less problems than others, still not many problems reported 😬

@Finii Finii merged commit f88122f into master Apr 10, 2024
6 checks passed
@Finii Finii deleted the bugfix/fa-reorder branch April 10, 2024 06:03
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 this pull request may close these issues.

Patcher overrides Fira Code progress bars and spinners in fonts Some update changed PROPO glyphs size
2 participants