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

Feature: stable codepoints #778

Merged
merged 3 commits into from
Sep 23, 2022
Merged

Feature: stable codepoints #778

merged 3 commits into from
Sep 23, 2022

Conversation

Finii
Copy link
Collaborator

@Finii Finii commented Feb 4, 2022

Description

The Font Logos and Octicons codepoints depend on the parallel existence
of FontAwesome. I.e. Font Logos is shifted of Octicons or FontAwesome is
also present in the patched font; Octicons is shifted if FontAwesome is
present.

This means that people, although using a Nerd Font, can expect the
symbols in different locations. The reason is clear; people that just
want one or some symbols and use a specifically patched font will be
able to use the original symbol font codepoints.

But I guess that these uses are nonexisting. Almost all will use
'complete' patched fonts and that is what we deliver and document.

To make the documentation less complicated we should fix the code point
ranges that a specific symbol set will be patched in at.

The two possible places where Octicons / Font Logos ends up are there
since they have been added back in 2015/6 (commits 9620d47, f933b5a).

Requirements / Checklist

What does this Pull Request (PR) do?

Make the codepoints of symbol sets stable, i.e. independent of other also incuded symbol sets.

It also drops SrcEnd from the patch table with 159820b. This improves the maintainability of the code and should be cherry-picked even if the complete PR is not merged.

How should this be manually tested?

Generate --complete font before and after this changes, fonts are identical (maybe use fontforge font compare, or cmp) (I did this).

Any background context you can provide?

What are the relevant tickets (if any)?

Came up with #762 i.e. on https://github.com/ryanoasis/nerd-fonts/wiki/Glyph-Sets-and-Code-Points.

Screenshots (if appropriate or helpful)

@Finii
Copy link
Collaborator Author

Finii commented Feb 4, 2022

Please consider 159820b independently, that commit just improves code maintainability with no functional change.

@Finii Finii mentioned this pull request Jul 7, 2022
2 tasks
@Finii Finii added this to the v2.3.0 milestone Aug 22, 2022
[why]
In patch set definitions we have for the source ranges SymStart and SymEnd
and for the destination we can specify SrcStart and SrcEnd

The SrcEnd can be automatically generated. For SrcEnd values that differ
from the autogenerated value (are lower) the script would crash, or (are
higher) ignore them anyhow.

There are two modes: 'exact = True' and 'exact = False'.

The SrcStart and SrcEnd values are ignored if exact is True, because the
glyphs are patched into the same codepoint where they originate. This
also means that gaps in the symbols are preserved - all patched in
glyphs have the same codepoint as they have in the source (symbol) font.

When exact is False on the other hand, all (non empty) glyphs are filled
into the codepoints that start at SrcStart. Gaps (empty glyphs) are
ignored and thus are not present as gap in the patched font anymore (*).
The to-be-filled-next codepoint in the patched font just increases by 1
on every filled symbol.

See note for the reason.

This also makes maintining the patch set easier as noone needs to
'calculate' a SrcEnd value anymore.

[how]
Use directly the start value and the counter instead of filling an array
with a range, that is then indexed by the counter.

Before this commit:

  list_of_patched_font_codepoints = list(range(SrcStart, SrcEnd + 1))
  current_codepoint = list_of_patched_font_codepoints[loop_counter]

After this commit:

  current_codepoint = SrcStart + loop_counter

[note]
Maybe related to code removed with c728079.

I guess the code uses the list, because it has been believed that glyphs
can only be indexed by strings containing a hex number. The array
supposedly contained that strings.

But in fact the fontforge python module docu is like this:

font.__getitem__(key)

    If key is an integer, then returns the glyph at that encoding.
    If a string then returns the glyph with that name.
    May not be assigned to.

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why]
The Font Logos and Octicons codepoints depend on the parallel existence
of FontAwesome. I.e. Font Logos is shifted of Octicons or FontAwesome is
also present in the patched font; Octicons is shifted if FontAwesome is
present.

This means that people, although using a Nerd Font, can expect the
symbols in different locations. The reason is clear; people that just
want one or some symbols and use a specifically patched font will be
able to use the original symbol font codepoints.

But I guess that these uses are nonexisting. Almost all will use
'complete' patched fonts and that is what we deliver and document.

To make the documentation less complicated we should fix the code point
ranges that a specific symbol set will be patched in at.

[how]
Just drop the associated functions and use a False constant instead.

[note]
The two possible places where Octicons / Font Logos ends up are there
since they have been added back in 2015/6 (commits 9620d47, f933b5a).

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why]
The patch set table has 'contradicting' or 'complicated' entries.

[how]
When we use exact patching a SrcStart will be ignored and shall be None
to make that clear.

The other two cases patch in only one glyph, make the entries more easy
we could either make them 'exact' (reuse source codepoint) or specify a
SrcStart. At the moment they rely on the (hidden?) rule that non-exact
entries without SrcStart still reuse the SymStart...

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
@Finii Finii merged commit 3b57d3b into master Sep 23, 2022
@Finii Finii deleted the feature/stable-codepoints branch September 23, 2022 09:55
LNKLEO pushed a commit to LNKLEO/Nerd that referenced this pull request Nov 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant