gnovm: add -X flag to gno run and gno test#5985
Conversation
🛠 PR Checks Summary🔴 Pending initial approval by a review team member, or review from tech-staff Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):🟢 Maintainers must be able to edit this pull request (more info) ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
notJoon
left a comment
There was a problem hiding this comment.
I don't think using a regex is a good or safe approach, because it would replace not only the variable names but also all raw strings in the code.
the original issue requires support for build, test and publish but it doesn't look like all of those requirements have been addressed. so I think this is not acceptable in its current state
Adds a -X name=value flag (repeatable) to 'gno run' and 'gno test', mirroring 'go build -ldflags "-X pkg.name=value"'. Rewritten from an earlier regex-based approach per review feedback (PR gnolang#5985 / notJoon): a text-based regex can't distinguish a real top-level var declaration from text that merely looks like one, e.g. inside a backtick-quoted raw string literal elsewhere in the file. This version parses the source with go/parser, walks only the file's top-level Decls (so it never descends into function bodies, and never touches local variables or string-literal *contents*), finds package-level 'var name = "..."' declarations (including grouped 'var (...)' blocks, and both quoted and raw-string initializers) whose name matches an override, replaces the literal's Value via strconv.Quote, and re-serializes the file with go/printer. Only top-level var (never const) declarations are eligible, matching go build -X's own semantics. Note: 'gno build' and 'gno publish' don't currently exist as separate gno CLI subcommands (only run/test/lint/etc. do), so this covers the two existing commands that parse and execute .gno source directly; see the PR discussion for more on this. Added a dedicated regression test for the exact scenario from review (a raw string containing text that resembles a var declaration must be left untouched), plus tests for grouped var blocks, const declarations being skipped, and function-local variables being left alone. Closes gnolang#1021
6ebb08e to
7f7b0e8
Compare
…nario The previous version of this test only had ONE actual var declaration (whose value happened to be a raw string containing fake-looking declaration text), so patching that single declaration's entire value -- including the embedded fake text -- was correct behavior, not a bug. The test's assertion that the fake text should survive was wrong. Rewritten with two separate declarations: a real "var myVar = ..." to patch, and a different "var otherVar = <raw string>" whose raw string value merely contains text resembling a var myVar declaration. This actually exercises the scenario from review: only the real declaration is patched, and otherVar's raw string content is preserved byte-for-byte.
29e5d4e to
639f73f
Compare
|
Thanks for the review, both points are addressed:
|
Adds a -X name=value flag (repeatable) to 'gno run' and 'gno test', mirroring 'go build -ldflags "-X pkg.name=value"'. Before parsing, matching package-level (unindented) 'var name = "..."' or 'var name string = "..."' declarations have their string literal initializer replaced with the given override value.
Note: 'gno build' and 'gno publish' do not currently exist as separate gno CLI subcommands (only run/test/lint/etc. do), so this covers the two existing commands that parse and execute .gno source directly.
Closes #1021