Skip to content

Add Luax.set-string-global#3

Merged
hellerve merged 1 commit into
masterfrom
claude/luax-set-string-global
Jun 1, 2026
Merged

Add Luax.set-string-global#3
hellerve merged 1 commit into
masterfrom
claude/luax-set-string-global

Conversation

@carpentry-agent
Copy link
Copy Markdown

The set-*-global family in Luax had int, float, and bool variants but was missing string. This meant Luax.set-string-global didn't exist despite Luax.get-string-global, all set-*-field variants, and the Lua.set-string-global docs all pointing users toward it.

Adds (luax--def-set-global string Lua.push-carp-str) alongside the existing three variants, plus tests for the new function.

Also updated existing tests that used Lua.set-string-global to use Luax.set-string-global where appropriate.


Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

The set-*-global family had int, float, and bool variants but was
missing string. This adds the missing macro expansion and tests.
Copy link
Copy Markdown

@carpentry-reviewer carpentry-reviewer Bot left a comment

Choose a reason for hiding this comment

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

Build & Tests

CI passes on both macOS and Ubuntu. No local Carp compiler on this machine — verified via CI only.

Findings

No issues found. This is a clean, mechanical addition that follows the exact pattern of the existing set-int-global, set-float-global, and set-bool-global variants.

  • lua.carp:526(luax--def-set-global string Lua.push-carp-str) mirrors the set-*-field family at line 546 which already has a string variant using the same pusher. Consistent.
  • The macro at line 42 expands to the same logic as the hand-written Lua.set-string-global (line 299), but scoped to Luax — no naming conflict.
  • Tests are thorough: basic set/get round-trip plus a cross-boundary test that sets a global from Carp and reads it back through a Lua expression (do-string).
  • The two test migrations from Lua.set-string-globalLuax.set-string-global (lines 249, 279) are correct — those tests exercise Luax.get-*-global and should use the Luax setter for consistency.

Verdict: merge

Straightforward gap-fill. Code is correct, tests are good, CI is green.

@hellerve hellerve merged commit bb755a7 into master Jun 1, 2026
2 checks passed
@hellerve hellerve deleted the claude/luax-set-string-global branch June 1, 2026 11:36
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