From ea5171ae783ccf11fbb61627c452c2de293a4c57 Mon Sep 17 00:00:00 2001 From: Julian Noble Date: Sat, 4 Jul 2026 20:11:18 +1000 Subject: [PATCH] AGENTS: strengthen PUNKARGS documentation rules + add argument order guidance + interp_sync tests Root AGENTS.md closeout now includes a step to re-check modified .tm files for new procs lacking PUNKARGS argdoc blocks. src/modules/AGENTS.md Work Guidance explicitly requires argdoc blocks for all new procs in .tm files (documentation-only is acceptable). The Documentation section clarifies that 'user-facing' includes developer-facing utility procs. A new 'Argument Order In PUNKARGS' section explains the distinction between @leaders, @opts, and @values and why putting a leading argument in @values produces an incorrect synopsis. Verification section adds a check for PUNKARGS argdoc blocks on new procs. The interp_sync.test file contains 6 tcltest tests for the new punk::lib helper procs. --- AGENTS.md | 5 +- src/modules/AGENTS.md | 13 ++ .../punk/lib/testsuites/lib/interp_sync.test | 165 ++++++++++++++++++ 3 files changed, 181 insertions(+), 2 deletions(-) create mode 100644 src/tests/modules/punk/lib/testsuites/lib/interp_sync.test diff --git a/AGENTS.md b/AGENTS.md index d0b3fda7..11153c2b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,8 +71,9 @@ Default section order: 2. Update nearest owning docs and any affected parents or children 3. Refresh every affected Child DOX Index 4. Remove stale or contradictory text -5. Run existing verification when relevant -6. Report any docs intentionally left unchanged and why +5. Re-check modified `.tm` files for new procs lacking PUNKARGS `argdoc` blocks +6. Run existing verification when relevant +7. Report any docs intentionally left unchanged and why ## User Preferences diff --git a/src/modules/AGENTS.md b/src/modules/AGENTS.md index e763da86..ee75ab3c 100644 --- a/src/modules/AGENTS.md +++ b/src/modules/AGENTS.md @@ -43,6 +43,7 @@ Source of truth for all editable Punk project modules. This is where agents shou - If the same proc exists in both `src/modules/` and `src/modules_tcl/`, prefer `src/modules/` unless version-specific behavior is relevant or only the version-specific file is active. - Use `deck module.new ` or `punk::mix::commandset::module::new` to scaffold new modules. - Run `tclsh src/make.tcl modules` to build modules, or `tclsh src/make.tcl project` for a full build. +- When adding a new proc to a `.tm` file, add a PUNKARGS `argdoc` block immediately before it — even if the proc parses arguments manually and the PUNKARGS is documentation-only. "User-facing" includes any proc in an exported namespace, not just shell commands. Developer-facing utility procs (e.g `punk::lib::*`) are not exempt. ### Formatting And Layout @@ -168,6 +169,16 @@ dict create {*}[ Update the template with concrete details whenever functions are user-facing or complex. +### Argument Order In PUNKARGS + +The `@leaders`, `@opts`, and `@values` sections are not interchangeable — they determine the position of arguments in the synopsis and the expected parsing order: + +- **`@leaders`** — arguments that appear before opts (e.g `interp` in `interp_sync_package_paths interp -libunknown 1`). Use these when the proc signature has a fixed leading argument before optional flags, even if the proc parses args manually with `{interp args}`. +- **`@opts`** — flags that appear after leaders (and before values). +- **`@values`** — trailing arguments that appear after opts. + +Putting a leading argument in `@values` instead of `@leaders` produces an incorrect synopsis (e.g `[-libunknown ] interp` instead of `interp [-libunknown ]`). Match the PUNKARGS section to the actual call site order, not just to which arguments are "required" vs "optional". + ### Error Handling And Logging - Prefer `try { ... } on error {result options} { ... }` for structured Tcl 8.6+ error handling. @@ -317,6 +328,7 @@ return ### Documentation And Comments - Primary documentation for procs should use the `punk::args` system, preferably within an `argdoc` sub-namespace using `lappend PUNKARGS `. +- "User-facing" in the Procedure Documentation Template Guide means any proc in an exported namespace or any proc a developer might call — not just shell commands. Developer-facing utility procs are not exempt. - If a public proc parses arguments manually, keep the implementation behavior synchronized with its PUNKARGS definition. - Update relevant docs or usage notes when behavior changes. - Mention environment variables or flags required to run new features. @@ -324,6 +336,7 @@ return ## Verification - VS Code Tcl lint diagnostics are clean for modified `.tm` files when available. +- New procs in modified `.tm` files have PUNKARGS `argdoc` blocks (documentation-only is acceptable if the proc parses args manually). - `tclsh src/make.tcl modules` completes without errors for module changes. - Module tests pass: `tclsh src/tests/modules//tests/all.tcl`. diff --git a/src/tests/modules/punk/lib/testsuites/lib/interp_sync.test b/src/tests/modules/punk/lib/testsuites/lib/interp_sync.test new file mode 100644 index 00000000..c5d39e3c --- /dev/null +++ b/src/tests/modules/punk/lib/testsuites/lib/interp_sync.test @@ -0,0 +1,165 @@ +package require tcltest +package require punk::lib + +namespace eval ::testspace { + namespace import ::tcltest::* + + # -------------------------------------------------------------------------- + # Test 1: Basic propagation — tm list, auto_path, package prefer + # -------------------------------------------------------------------------- + test interp_sync_basic {interp_sync_package_paths propagates tm list, auto_path, and package prefer to child interp}\ + -setup { + set saved_prefer [package prefer] + package prefer latest + set child [interp create] + }\ + -body { + punk::lib::interp_sync_package_paths $child + set result [list] + lappend result [expr {[interp eval $child {tcl::tm::list}] eq [tcl::tm::list]}] + lappend result [expr {[interp eval $child {set ::auto_path}] eq $::auto_path}] + lappend result [interp eval $child {package prefer}] + return $result + }\ + -cleanup { + interp delete $child + package prefer $saved_prefer + }\ + -result {1 1 latest} + + # -------------------------------------------------------------------------- + # Test 2: package prefer latest enables alpha version selection in child + # -------------------------------------------------------------------------- + test interp_sync_prefer_latest_version {package prefer latest propagated to child enables alpha version selection}\ + -setup { + set saved_prefer [package prefer] + package prefer latest + namespace eval ::testsynthpkg {} + package ifneeded testsynthpkg 0.1.1 {package provide testsynthpkg 0.1.1} + package ifneeded testsynthpkg 999999.0a1.0 {package provide testsynthpkg 999999.0a1.0} + set child [interp create] + punk::lib::interp_sync_package_paths $child + #register both versions in child + interp eval $child {namespace eval ::testsynthpkg {}} + interp eval $child [list package ifneeded testsynthpkg 0.1.1 {package provide testsynthpkg 0.1.1}] + interp eval $child [list package ifneeded testsynthpkg 999999.0a1.0 {package provide testsynthpkg 999999.0a1.0}] + }\ + -body { + interp eval $child {package require testsynthpkg} + }\ + -cleanup { + interp delete $child + package forget testsynthpkg + package prefer $saved_prefer + }\ + -result {999999.0a1.0} + + # -------------------------------------------------------------------------- + # Test 3: Ordering preservation — tm list and auto_path must match exactly + # -------------------------------------------------------------------------- + test interp_sync_ordering {tm list and auto_path ordering preserved exactly in child interp}\ + -setup { + set child [interp create] + }\ + -body { + punk::lib::interp_sync_package_paths $child + set parent_tm [tcl::tm::list] + set child_tm [interp eval $child {tcl::tm::list}] + set parent_ap $::auto_path + set child_ap [interp eval $child {set ::auto_path}] + #compare element-by-element + set tm_match [expr {[llength $parent_tm] == [llength $child_tm]}] + for {set i 0} {$tm_match && $i < [llength $parent_tm]} {incr i} { + if {[lindex $parent_tm $i] ne [lindex $child_tm $i]} { + set tm_match 0 + } + } + set ap_match [expr {[llength $parent_ap] == [llength $child_ap]}] + for {set i 0} {$ap_match && $i < [llength $parent_ap]} {incr i} { + if {[lindex $parent_ap $i] ne [lindex $child_ap $i]} { + set ap_match 0 + } + } + return [list $tm_match $ap_match] + }\ + -cleanup { + interp delete $child + }\ + -result {1 1} + + # -------------------------------------------------------------------------- + # Test 4: snapshot_package_paths returns a valid script string + # -------------------------------------------------------------------------- + test snapshot_package_paths_valid {snapshot_package_paths returns a script that reproduces state when eval'd in child}\ + -setup { + set saved_prefer [package prefer] + package prefer latest + set child [interp create] + }\ + -body { + set script [punk::lib::snapshot_package_paths] + #verify the script is non-empty and contains key commands + set has_tm [expr {[string first "tcl::tm::add" $script] >= 0}] + set has_ap [expr {[string first "set ::auto_path" $script] >= 0}] + set has_prefer [expr {[string first "package prefer" $script] >= 0}] + #eval in child and verify state matches + interp eval $child $script + set tm_match [expr {[interp eval $child {tcl::tm::list}] eq [tcl::tm::list]}] + set ap_match [expr {[interp eval $child {set ::auto_path}] eq $::auto_path}] + set prefer_match [expr {[interp eval $child {package prefer}] eq [package prefer]}] + return [list $has_tm $has_ap $has_prefer $tm_match $ap_match $prefer_match] + }\ + -cleanup { + interp delete $child + package prefer $saved_prefer + }\ + -result {1 1 1 1 1 1} + + # -------------------------------------------------------------------------- + # Test 5: -libunknown 1 sets up epoch and package unknown handler in child + # -------------------------------------------------------------------------- + test interp_sync_libunknown {-libunknown 1 sets up punk::libunknown epoch and custom package unknown in child}\ + -setup { + set child [interp create] + }\ + -body { + punk::lib::interp_sync_package_paths $child -libunknown 1 + set result [list] + #epoch should exist in child + lappend result [interp eval $child {info exists ::punk::libunknown::epoch}] + #package unknown should be set to libunknown's handler (not default) + set unknown_handler [interp eval $child {package unknown}] + lappend result [expr {[string first "punk::libunknown" $unknown_handler] >= 0}] + return $result + }\ + -cleanup { + interp delete $child + }\ + -result {1 1} + + # -------------------------------------------------------------------------- + # Test 6: Regression — existing interp_sync_package_paths without -libunknown + # -------------------------------------------------------------------------- + test interp_sync_regression {interp_sync_package_paths without -libunknown still propagates auto_path, tm list, and package prefer}\ + -setup { + set saved_prefer [package prefer] + package prefer latest + set child [interp create] + }\ + -body { + punk::lib::interp_sync_package_paths $child + set result [list] + lappend result [expr {[interp eval $child {set ::auto_path}] eq $::auto_path}] + lappend result [expr {[interp eval $child {tcl::tm::list}] eq [tcl::tm::list]}] + lappend result [interp eval $child {package prefer}] + return $result + }\ + -cleanup { + interp delete $child + package prefer $saved_prefer + }\ + -result {1 1 latest} + +} + +tcltest::cleanupTests