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

[types] Add Tag::NULL associated constant #1339

Merged
merged 1 commit into from
Feb 4, 2025
Merged

[types] Add Tag::NULL associated constant #1339

merged 1 commit into from
Feb 4, 2025

Conversation

cmyr
Copy link
Member

@cmyr cmyr commented Feb 3, 2025

We'd already decided to use this value (four spaces) as the value of Tag::default(), and it turns out that it is also allowed in OS/2, so exposing it as a constant seems useful.

We'd already decided to use this value (four spaces) as the value of
Tag::default(), and it turns out that it is also allowed in OS/2, so
exposing it as a constant seems useful.
@anthrotype anthrotype merged commit ee7faf6 into main Feb 4, 2025
10 checks passed
@anthrotype anthrotype deleted the null-tag branch February 4, 2025 09:27
@behdad
Copy link
Contributor

behdad commented Feb 4, 2025

At the risk of bikeshedding, I suggest renaming this to ::Spaces. At least in HB we use NONE for all-zero tag, to specify lack of a tag...

@anthrotype
Copy link
Member

I merged too quickly sorry. I kind of agree with behdad, calling this 'null' implies it uses null byte (0x0) which this is not.

Since the bike is already out of the shed, I suggest Tag::BLANK to match the description in the OS/2 table

This field can also be left blank (set to null, or a tag comprised of four space characters)

Though technically there isn't any semantical difference between all-zero and all-space tag, both mean the absence of a tag.

@cmyr
Copy link
Member Author

cmyr commented Feb 4, 2025

okay so maybe I misread that comment?

I heard "set to null, equivalent to four blank spaces", but maybe it means "set to all zeros, or to four blank spaces"?

@cmyr
Copy link
Member Author

cmyr commented Feb 4, 2025

in any case this constant isn't really doing anything, so happy to just revert...

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.

4 participants