Skip to content

Ability to create a PString programmatically#1480

Open
timboudreau wants to merge 1 commit intogooglefonts:mainfrom
timboudreau:pstring_constructor
Open

Ability to create a PString programmatically#1480
timboudreau wants to merge 1 commit intogooglefonts:mainfrom
timboudreau:pstring_constructor

Conversation

@timboudreau
Copy link
Copy Markdown

This library seems oriented toward modifying font files, but some of us are insane enough to want to create one programmatically from scratch. That requires being able to populate strings programmatically from scratch, but there is no clean way to construct a PString.

There is no obvious way to construct a PString from a str or String. While it is possible to hack around that by putting the desired string in quotes and then using serde_json to deserialize it, that is a pretty ugly solution.

This patch simply adds a From<I : Into<String>> implementation for it.

This library seems oriented toward *modifying* font files, but some of us are insane enough to
want to create one programmatically from scratch.  That requires being able to populate strings
programmatically from scratch, but there is no clean way to construct a `PString`.

There is no obvious way to construct a `PString` from a `str` or `String`.  While it is
possible to hack around that by putting the desired string in quotes and then using `serde_json`
to deserialize it, that is a pretty ugly solution.

This patch simply adds a `From<I : Into<String>>` implementation for it.
@google-cla
Copy link
Copy Markdown

google-cla Bot commented Apr 26, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@timboudreau
Copy link
Copy Markdown
Author

I have filed a CLA. Not sure what's needed to retrigger the workflow here.

@nicoburns
Copy link
Copy Markdown
Contributor

I have filed a CLA. Not sure what's needed to retrigger the workflow here.

There's a "recheck" button if you click through to the CLA details. I've pressed it for you seeing as anyone can press it.

Copy link
Copy Markdown
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

So this crate is designed to be used for programatically creating fonts (we are building a font compiler on top of it) but we haven't needed a constructor for this type because we expect the user to use the new_v2 convenience method to build a full valid post table.

The advantage of this is that it makes sure the generated table is well-formed.

That said, a constructor is fine! But along side From can we also add a PString::new method? It can have the same signature.

#[test]
fn test_pstring_dref() {
let string = PString::from("Hello");
assert_eq!("Hello", std::ops::Deref::deref(&string));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

fwiw the deref isn't necessary, we have a partialeq for &str.

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.

3 participants