Skip to content

feat: add object-form table factory API - #135

Open
tecfu wants to merge 3 commits into
masterfrom
feat/object-table-factory
Open

tecfu wants to merge 3 commits into
masterfrom
feat/object-table-factory

Conversation

@tecfu

@tecfu tecfu commented Oct 6, 2026

Copy link
Copy Markdown
Owner

Add Table.create({ header, rows, footer, options }) as a clearer extension point while preserving every existing positional overload. This gives future API additions a named home without breaking compatibility.

@tecfu

tecfu commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Review: merge (one note)

Table.create dispatches onto the exact positional shapes the existing factory accepts — same Factory entry point, no duplicated parsing — and every positional overload is untouched. Verified: Table.create({header, rows, options}) matches positional Table(header, rows, options) output byte-for-byte; the footer and rows-only branches hit Factory the same way; matrix 0; 111/111 vitest; 21/21 goldens; typecheck/lint clean.

Two details that I checked because they'd be the obvious bugs:

  • create({ rows }) with no header: header.length is 0 so it takes the 2-arg path (Factory([rows, options])), matching positional. ✅
  • The options parameter for the 3/4-arg positional paths is in the right slot for each branch. ✅

1 note: the test uses expect(table.render()).toContain("Ada") — weak: any output containing "Ada" passes (even a header-dropped or row-shifted bug would), and it doesn't assert that the options were honored. Since this is the only coverage of a new public entry point, worth strengthening to (a) byte-equality with the positional equivalent, and (b) something that proves options reached the renderer (e.g. an assert that the 30-wide option constrains the line width).

Sincerely qwen3.8-flash-next

tecfu commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Review: merge, with test/documentation note

The implementation is clean and preserves the existing positional factory paths by dispatching directly to the same Factory entry point. The three dispatch cases are correctly shaped.

One thing I would still tighten before merge: the only new behavioral test asserts merely that the rendered output contains Ada. That does not prove the object-form API selected the same overload or that options were actually applied. Please make this a parity test against the equivalent positional call, ideally byte-for-byte, and assert one observable option effect such as width. Since Table.create is a new public API, a short README/type-level usage example would also be appropriate.

No architectural blocker found.

— GPT-5.6 Luna

tecfu commented Oct 7, 2026

Copy link
Copy Markdown
Owner Author

Re-review: test parity fixed

The object-form factory test now compares complete rendered output against the equivalent positional factory call, verifying dispatch and option propagation.

CI is running on the updated test. No implementation blocker found.

— GPT-5.6 Luna

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.

1 participant