Skip to content

gamut-iptc: should an extension structure's retained fields be a public field or reached through accessors? #591

Description

@justin13888

Each typed structure in gamut_iptc::extension carries pub other: Vec<XmpProperty>: every field
the typed read took no value from, kept verbatim so a read-modify-write drops nothing. Seven types
now expose it (CreatorContactInfo, ArtworkOrObject, Licensor, ImageRegion, RegionBoundary,
RegionBoundaryPoint, Entity).

It is a public field of Vec<XmpProperty>, and that is the open question.

Against a public field

  • AGENTS.md asks that the public API stay mechanically portable to C: configs are "plain data —
    Copy structs, fieldless enums ... or payloads reachable through accessors". A Vec of a
    recursive value tree is exactly the payload an accessor pair is meant to cover.
  • It is writable, so a caller can put anything in it, including a name the model itself owns. The
    emission rule handles that today (a retained field is emitted only when no modelled field claims
    its name), but the rule exists because the collection is reachable.

For leaving it public

  • Every other field of these structures is public plain data, and the types are
    #[non_exhaustive], so a later move to accessors is not a breaking change for construction —
    callers already build from Default and assign.
  • A borrowed accessor pair (other() / other_mut()) buys nothing over a public field while the
    element type is XmpProperty, which is itself a public struct with public fields.

What to decide

  1. Leave other public and document it as the retention slot (status quo).
  2. Replace it with other() / push_other() / set_other() across the seven types, and say what
    the accessor rejects (a field whose name the model owns? one with an empty name?).
  3. Something narrower — e.g. keep the field public but make the element type opaque.

Whichever is chosen should be applied to all seven types at once, and to whatever the remaining
Extension structures (#538) grow.

Raised by the review of #540, which fixed the emission rule that made a writable retention list
hazardous; the API question itself was left open there deliberately.

Refs #422

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions