googlefonts / googlefonts/fontations

Declare structs for types with more than a single `ReadArgs` argument

Open
#108 2 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Rust
Stars
826
Forks
75
Avg merge
22h 33m
Merged PRs (30d)
75

Description

Currently if a type has multiple read arguments, we pass them in as a tuple, but this makes it very easy to lose track of them, especially if the arguments are of the same type. It would be nice if instead of this, we declared a struct with named fields, and used that.

In this world,

```rust
#[read_args(number_of_h_metrics: u16, num_glyphs: u16)]
table Hmtx {
// ...
}
```

would produce:

```rust
pub struct HmtxArgs {
pub number_of_h_metrics: u16,
pub num_glyphs: u16,
}

impl ReadArgs for Hmtx {
type Args = HmtxArgs;
}

impl<'a> FontReadWithArgs<'a> for Hmtx<'a> {
fn read_with_args(data: FontData<'a>, args: &HmtxArgs) -> Result {
let HmtxArgs { number_of_h_metrics, num_glyphs } = *args;
// ..
}
}
```

instead of,

```rust
impl ReadArgs for Hmtx {
type Args = (u16, u16);
}

impl<'a> FontReadWithArgs<'a> for Hmtx<'a> {
fn read_with_args(data: FontData<'a>, args: &(u16, u16)) -> Result {
let (number_of_h_metrics, num_glyphs) = *args;
}
}
```

which should be much easier to understand.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by locating the read_args attribute, ReadArgs, and FontReadWithArgs implementations described in the issue. Trace how multiple arguments are currently represented as tuples, then determine the generated struct and destructuring behavior needed for named fields. Done means multi-argument types use generated named argument structs instead of tuples, with existing behavior preserved.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.