Browse Source
textblock::table uses an unvalidated dict merge with no choice validation; its PUNKARGS is incomplete and the -return -choiceprefix documentation mismatch is the last open item from the punk::args -choices audit. This goal proposes refactoring textblock::class::table's constructor to carry inline punk::args define blocks (the class_ansi pattern) so the constructor's documented option set becomes the single source of truth that textblock::table's PUNKARGS references rather than a parallel hand-typed list. Assisted-by: harness=opencode; primary-model=openrouter/z-ai/glm-5.2; api-location=openrouter.aimaster
2 changed files with 83 additions and 0 deletions
@ -0,0 +1,77 @@
|
||||
# G-048 textblock::table: parse via punk::args with shared passthrough documentation for table constructor options |
||||
|
||||
Status: proposed |
||||
Scope: src/modules/textblock-999999.0a1.0.tm (textblock::table proc + PUNKARGS, textblock::class::table class - constructor, opts_table_defaults, methods) |
||||
Acceptance: textblock::class::table's constructor carries an inline punk::args define block (lappend PUNKARGS, @id naming the class+method) and parses its args via punk::args::parse withid, replacing the current manual switch at L466; textblock::table's PUNKARGS references the constructor's documented options (via @id reference, a shared fragment, or a documented include mechanism - the chosen mechanism recorded in the detail file) so table's definition is complete without hand-duplicating the constructor's option list; an invalid -return value (e.g. -return tab) produces a punk::args usage error instead of silently passing through; valid inputs reach textblock::class::table new with behaviour parity to the current dict merge; the -choiceprefix 0 on -return is honoured by construction (punk::args::parse enforces it); the "NOTE: more options available - argument definition is incomplete" comment is removed; existing textblock test suites pass. |
||||
|
||||
## Context |
||||
|
||||
textblock::table is a wrapper proc that creates a textblock::class::table object, configures it with passthrough options, then adds columns and rows. Its PUNKARGS definition (L6359-6385) documents only -return, -rows and -headers, with a comment "NOTE: more options available - argument definition is incomplete". The proc body (L6386) does `dict merge $defaults $args` with no validation — the `#todo - use punk::args` comment at L6387 acknowledges the gap. |
||||
|
||||
This was identified as item #8 in the punk::args -choices audit: -return carries `-choices {table tableobject}` with -choiceprefix defaulting to true (prefix matching promised), but the manual dict merge parser accepts arbitrary values with no validation at all. |
||||
|
||||
The constructor of textblock::class::table (L445) parses its args with a manual `switch -- $k` (L466) against `o_opts_table_defaults` (sourced from the `opts_table_defaults` dict at L188-224). This dict is a plain defaults dict — it carries no type information, no choices, no help text, no -choiceprefix settings. It cannot serve as documentation. |
||||
|
||||
## Approach |
||||
|
||||
### Phase 1: Refactor textblock::class::table constructor to use punk::args |
||||
|
||||
Follow the inline pattern established in punk::ansi::class::class_ansi (L122-135 for rendertest, L158-173 for render_to_input_line): |
||||
|
||||
```tcl |
||||
# Inside oo::class create class_ansi ... { ;# textblock equivalent: oo::class create table |
||||
lappend ::textblock::class::PUNKARGS [list { |
||||
@id -id "::textblock::class::table constructor" |
||||
@cmd -name "textblock::class::table constructor" -help\ |
||||
"..." |
||||
@opts |
||||
-title -default "" -type string |
||||
-frametype -default "light" -type string |
||||
... |
||||
}] |
||||
constructor {args} { |
||||
set argd [punk::args::parse $args withid "::textblock::class::table constructor"] |
||||
... |
||||
} |
||||
``` |
||||
|
||||
The `opts_table_defaults` dict remains as the backing store for default values, but the punk::args definition becomes the authoritative documentation (types, choices, help text, -choiceprefix etc. that a plain dict cannot express). |
||||
|
||||
Key decisions to record in this file when made: |
||||
- Whether the constructor's define block is registered via `punk::args::define` (module-level) or left as a `lappend PUNKARGS` collected at load time (the class_ansi pattern uses `lappend` inside the class body — verify this is scanned by punk::args registration). |
||||
- How the `configure`/`cget` methods (which also reference `o_opts_table_defaults`) relate to the constructor's define block — they may get their own inline blocks or share a fragment. |
||||
|
||||
### Phase 2: textblock::table PUNKARGS references the constructor's definition |
||||
|
||||
textblock::table's PUNKARGS will include its wrapper-specific options (-return with -choiceprefix 0, -rows, -headers) plus a reference to the constructor's documented options so the full passthrough surface is visible in table's own synopsis/help without hand-duplicating. |
||||
|
||||
Candidate mechanisms (to be evaluated during implementation): |
||||
1. **@id reference / include** — if punk::args supports referencing another definition's @opts block (or a named fragment) to inline it. Check whether `punk::args::resolved_def` or a similar mechanism can pull opts from another @id. |
||||
2. **Shared fragment** — a named PUNKARGS fragment (like `::punk::console::argdoc::console_opts`) that both the constructor and the table wrapper reference. This is the pattern used by punk::console for the -console option. |
||||
3. **Manual enumeration with a maintenance guard** — if no include mechanism exists, hand-list the passthrough opts in table's PUNKARGS with a comment pointing to the constructor's @id as the source of truth, and a note that the two must be kept synchronized. This is the fallback if punk::args has no reference/include capability yet. |
||||
|
||||
The chosen mechanism goes here when decided. |
||||
|
||||
### Phase 3: Replace dict merge with punk::args::parse in textblock::table |
||||
|
||||
```tcl |
||||
proc table {args} { |
||||
set argd [punk::args::parse $args withid ::textblock::table] |
||||
set opts [dict get $argd opts] |
||||
... |
||||
} |
||||
``` |
||||
|
||||
The `-return` option's `-choiceprefix 0` is enforced by punk::args::parse by construction — no manual prefix handling needed. |
||||
|
||||
## Alternatives considered |
||||
|
||||
- **Add tcl::prefix::match to the existing dict merge** (as done for project::new in G-048-adjacent work) — rejected because the `#todo - use punk::args` comment shows the intent was always to switch to punk::args, and the constructor refactoring makes the passthrough documentation shareable which a prefix-match patch would not. |
||||
- **Leave table as manual parsing, add -choiceprefix 0 to PUNKARGS only** (the approach taken for overtype/punkcheck/nav items) — rejected because the dict merge accepts arbitrary values with no validation at all, and the user requested punk::args parsing for this case. Also, the constructor refactoring opportunity (inline define blocks) makes the full fix more valuable than a doc-only patch. |
||||
|
||||
## Notes |
||||
|
||||
- The punk::ansi::class::class_ansi inline pattern (L122-173) is the reference implementation for class-level punk::args define blocks. |
||||
- opts_table_defaults (L188-224) contains ~20 options including framing, ANSI base/border, show flags, and width limits. Not all may need full -type/-choices/-help documentation in phase 1 — the acceptance criteria requires at least the major ones (-title, -frametype, -show_header, -ansiborder) with the rest covered by a documented passthrough mechanism. |
||||
- The constructor has a special case at L454-456: a single-element args list is treated as a title. This must be preserved in the punk::args definition (likely via a @leaders form or a -default on -title with appropriate -min). |
||||
- Related audit items already fixed: console_fact_get/set (-choiceprefix 0), overtype renderspace/renderline (-choiceprefix 0), nav::fs::d/ (-choiceprefix 0), punkcheck::install (-choiceprefix 0), project::new (tcl::prefix::match added). This goal closes the last remaining audit item (#8). |
||||
Loading…
Reference in new issue