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

Update octicons and drop old MDI #1156

Merged
merged 3 commits into from
Apr 24, 2023
Merged

Update octicons and drop old MDI #1156

merged 3 commits into from
Apr 24, 2023

Conversation

Finii
Copy link
Collaborator

@Finii Finii commented Mar 25, 2023

This can only be done after MDI-old has been dropped

[why]
The octicons got a lot updates.
But they do not have a font anymore.

[how]
Keep our old codepoints constant, but add the new icons thereafter.

Fixes: #490

Requirements / Checklist

What does this Pull Request (PR) do?

Update the Octicons icons.

If one icon has been dropped (without replacement) re-use its codepoint for a newly added icon.
Append other newly added icons in the end.
Afterwards Octicons will be gapless in F400 - F532 in Nerd Fonts.

Note that we reassign codepoints on patching. The original octicons font has many gaps.
To keep also compatible here the font we create here is a bit ... stranger, with a lot of gaps and some gaps filled.
The methodology seems complicated by the goal is easy: We just want to patch in the icon into Nerd Fonts with the aformentioned codepoints and no additional special rules.

This means for the octicons.ttf
Existing gaps are preserved.
Dropped icons are replaced by newly added ones.
The remainder of new icons is added in the back, which is FAR back, because of the one stray icon the original (old) font had.

How should this be manually tested?

Any background context you can provide?

What are the relevant tickets (if any)?

Screenshots (if appropriate or helpful)

@Finii Finii added this to the v3.0.0 milestone Mar 25, 2023
@Finii
Copy link
Collaborator Author

Finii commented Mar 25, 2023

Current output:

Reading mapping file
Found 172 entries
Fetching octicons archive "v18.2.0.tar.gz"


Unpacking octicons archive
Found 574 svgs
Found 308 icons after de-duplicating

Renamed cloud-download -> download
Renamed cloud-upload -> upload
Renamed clippy -> paste
Renamed mail-read -> read
Renamed primitive-dot -> dot-fill
Renamed primitive-square -> square-fill
Renamed settings -> sliders
Renamed dashboard -> meter
Renamed trashcan -> trash
Renamed paintcan -> paintbrush

Missing octoface
Missing keyboard
Missing gist
Missing file-text
Missing file-pdf
Missing jersey
Missing radio-tower
Missing repo-force-push
Missing mail-reply
Missing arrow-small-right
Missing gist-secret
Missing no-newline
Missing arrow-small-up
Missing arrow-small-down
Missing arrow-small-left
Missing file-symlink-directory
Missing circuit-board
Missing watch
Missing text-size
Missing ellipses
Missing plus-small

Found 151 (of 172, missing 21) and new 157

I guess the icons that have been dropped are no problem (but then, what do I know what people use these icons for).
I would just drop them, update the rest, fill in the new gaps with new icons and append the rest of new icons. As they have no codepoints yet we can assign whatever we want.

@Finii
Copy link
Collaborator Author

Finii commented Mar 26, 2023

Octicons have 2 sets of icons, one for 16px and one for 24px scaling. Unfortunately not all icons are available in both sizes.

Whatever, here a comparison of old (left) versus new (right)

image

The old size seems to be in-between, stroke size wise:

image

@Finii
Copy link
Collaborator Author

Finii commented Mar 26, 2023

@jef @djensenius What do you think, should we use the 16px or the 24 px octicons?
Given terminal font size 11-15 pt I would guess the 16 px size is better (at the usual 96 dpi)

@Finii Finii force-pushed the feature/update-octicons branch 4 times, most recently from 673cf75 to c8fa239 Compare March 26, 2023 09:45
@Finii
Copy link
Collaborator Author

Finii commented Mar 26, 2023

Fixed copyright issue, force push (sorry for the frequent pushes)

@Finii
Copy link
Collaborator Author

Finii commented Mar 26, 2023

We need to wait until Material Design Icons moved out of the way, because we need their old (dropped with v3.0.0) codepoints partially (F000 - F532).

We just need to do fontforge generate in src/glyphs/octicons/ and commit the changes as commit 2/2.

@jef
Copy link

jef commented Mar 26, 2023

@jef @djensenius What do you think, should we use the 16px or the 24 px octicons?
Given terminal font size 11-15 pt I would guess the 16 px size is better (at the usual 96 dpi)

Great point. IIRC, the 24px octicons have some that aren't available in 16px or vice versa.

If not, then 16px sounds great!

@Finii
Copy link
Collaborator Author

Finii commented Mar 26, 2023

Thanks for the reply. I had the same feeling, so be it. 👍

Great point. IIRC, the 24px octicons have some that aren't available in 16px or vice versa.

Both. The generate script builds a list of all icons, regardless of size. Then later it tries to find that icon in the preferred size and if not available falls back to the other size.

From image above, note 16px icons fill in where not 24px icons are available:
Screenshot 2023-03-26 at 22 57 11

@jef
Copy link

jef commented Apr 13, 2023

Amazing! Nice work and thanks for all the updates :)

@Finii
Copy link
Collaborator Author

Finii commented Apr 24, 2023

image

Right! 🤦‍♀️

This PR is just preparatory and commit 1/2. Needs to be completed!

@Finii Finii changed the title Update octicons Update octicons and drop old MDI Apr 24, 2023
[why]
The octicons got a lot updates.
But they do not have a font anymore.

[how]
Keep our old codepoints constant, but add the new icons thereafter.

This commit just moves all the mechanics in and moves the (old) font.
No actual update here.

The mapping file has been created with the analyze_octicons script.

Fixes: #490

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
This commit does the actual update of the octicons.ttf font file.
The script to update is 'generate'. It uses the mapping file from the
previous commit to create a new compatible font.

Some icons have meanwhile been dropped. We use their codepoints for new
icons.

Also fix a little bug in the GlyphInfo writer.

Following the output of the actual script run.

$ ./generate                                                                                                                                                                                                                                        feature/update-octicons ● 1 … 5 ⚑ 5 

Reading mapping file
Found 172 entries
Fetching octicons archive "v18.3.0.tar.gz"

  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100  550k    0  550k    0     0   504k      0 --:--:--  0:00:01 --:--:--  919k

Unpacking octicons archive
Found 576 svgs
Found 309 icons after de-duplicating

Renamed cloud-download -> download
Renamed cloud-upload -> upload
Renamed clippy -> paste
Renamed mail-read -> read
Renamed primitive-dot -> dot-fill
Renamed primitive-square -> square-fill
Renamed settings -> sliders
Renamed dashboard -> meter
Renamed trashcan -> trash
Renamed paintcan -> paintbrush

Missing octoface
Missing keyboard
Missing gist
Missing file-text
Missing file-pdf
Missing jersey
Missing radio-tower
Missing repo-force-push
Missing mail-reply
Missing arrow-small-right
Missing gist-secret
Missing no-newline
Missing arrow-small-up
Missing arrow-small-down
Missing arrow-small-left
Missing file-symlink-directory
Missing circuit-board
Missing watch
Missing text-size
Missing ellipses
Missing plus-small

Found 151 (of 172, missing 21) and new 158
Filled in missing, remaining new 137
Appended remaining new, total new mapping 309
Generating octicons.ttf with 309 glyphs
Generating GlyphInfo i_oct.sh
Finished

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why]
The old MDI are kept, just disabled. Maybe we need them or someone else,
for backward compatibility. Can be dropped sometime in the future.

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
@Finii Finii marked this pull request as ready for review April 24, 2023 19:30
@Finii Finii merged commit 1bb6814 into master Apr 24, 2023
@Finii Finii deleted the feature/update-octicons branch April 24, 2023 19:32
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.

feat: update Octicons to latest
2 participants