Skip to content

fix: consume implicit one-or-more placeholder in add - #395

Merged
webern merged 1 commit into
webern:mainfrom
rpatters1:fix/one-or-more-add-placeholder
Aug 15, 2026
Merged

fix: consume implicit one-or-more placeholder in add#395
webern merged 1 commit into
webern:mainfrom
rpatters1:fix/one-or-more-add-placeholder

Conversation

@rpatters1

Copy link
Copy Markdown
Contributor

Human Summary

While trying to build a fretboard diagram, discovered a bug in OneOrMore that could lead to an extra item being generated. This PR fixes that bug.

Summary

OneOrMore now tracks its implicit default placeholder. The first item added to a default-constructed or empty-repaired collection replaces that placeholder; explicitly supplied first items are preserved. Added core regression tests and removed the exporter-specific workaround.

Testing

  • Core unit tests: 221 assertions in 44 test cases
  • DirectionWriter tests: 30 assertions in 4 test cases
  • Changed files pass direct clang-format validation

References

  • No issue reference: GitHub issues are disabled for this repository.

@webern webern left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Interesting idea. It's not a big per-se, just a weird interface arising from the design goal of having invalid files be unrepresentable. This seems like a reasonable interface as well, I.e. the object takes care of it instead of the caller.

It feels like there might be other cases of this? Not to sure.

@rpatters1

Copy link
Copy Markdown
Contributor Author

I believe codex only found the one. (The output was a little confusing.) All the others seemed to be explicitly setting their first element, which this change accommodates.

@rpatters1

Copy link
Copy Markdown
Contributor Author

The "exporter-specific workaround" mentioned in the description was my initial fix for FrameData, never committed. I realized we probably needed a fix in OneOrMore.

@webern

webern commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Interesting idea. It's not a big per-se, just a weird interface arising from the design goal of having invalid files be unrepresentable. This seems like a reasonable interface as well, I.e. the object takes care of it instead of the caller.

It feels like there might be other cases of this? Not to sure.

Had opus open #396

@webern
webern merged commit 1a003aa into webern:main Aug 15, 2026
8 checks passed
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.

2 participants