Skip to content

Fix IDE companion template under the new compiler - #42

Open
struckchure wants to merge 2 commits into
vlang:masterfrom
struckchure:ide-companion-template
Open

struckchure wants to merge 2 commits into
vlang:masterfrom
struckchure:ide-companion-template

Conversation

@struckchure

Copy link
Copy Markdown

Summary

  • v -new-compiler -d ui2_custom_rendering run ./ide failed in the C stage with use of undeclared identifier 'err' / 'event'. companion_main_source in ide/model.v built the generated main.v as one interpolated string containing \${err} and \${event}; the new compiler doesn't honor the \$ escape inside an interpolated literal and emitted them as live interpolations.
  • Moved the template into a raw string with @PLACEHOLDER@ tokens filled by a single-pass replace_each, so the generated ${...} no longer depends on escaping.
  • User-supplied file/form names are now emitted as double-quoted literals, which vml_escape protects — a form named Mo's Form previously generated run_window('Mo's Form', ...), which doesn't compile.

Test plan

  • v -new-compiler -d ui2_custom_rendering run ./ide builds and launches
  • v -d ui2_custom_rendering run ./ide (old compiler) still builds
  • v -d ui2_custom_rendering test ide passes (it v -checks the generated companion)

Co-Authored-By: Claude noreply@anthropic.com

companion_main_source built the generated main.v as one interpolated
string that itself contained `\${err}` and `\${event}`. The new compiler
does not honor the `\$` escape there and emitted them as real
interpolations, so `v -new-compiler run ./ide` failed in the C stage
with undeclared `err` and `event`.

Move the template into a raw string with @Placeholder@ tokens filled by
a single-pass replace_each. User-supplied names are now emitted as
double-quoted literals, which vml_escape protects, so a form named
`Mo's Form` no longer produces uncompilable output.

Co-Authored-By: Claude <noreply@anthropic.com>

@medvednikov medvednikov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed the template rewrite together with vml_escape, the save/generation path, and the existing generated-companion test. I did not find a definite regression introduced by this patch. I left one non-blocking suggestion to preserve the escaping and placeholder behavior with automated regression coverage.

Validation: source-level review only; I could not independently run the old/new compiler builds in this environment. The PR's CI run is currently action_required, so it does not provide an independent passing result yet.

Comment thread ide/model.v
return "module main\n\nimport ui2\n\nconst ${const_name} = \$embed_file('${vml_escape(vml_name)}').to_string()\n\nfn build_form() ui2.Element {\n\treturn ui2.element_from_vml(${const_name}, ui2.bounds()) or {\n\t\teprintln('Could not load ${vml_escape(vml_name)}: \${err}')\n\t\tui2.screen(0x${app.form_background:06x}, [])\n\t}\n}\n\nfn handle_form_event(event string) {\n\tprintln('event: \${event}')\n}\n\nfn main() {\n\tui2.run_window('${vml_escape(app.form_name)}', ${int(app.form_width)}, ${int(app.form_height)}, build_form, handle_form_event)\n}\n"
// replace_each substitutes in a single pass, so a name containing another
// placeholder cannot be expanded a second time.
return companion_main_template.replace_each([

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Non-blocking: add regression coverage for the template's escaping and single-pass replacement

The existing test_saved_form_and_generated_main_compile_together only generates the default Form1 / form.vml case and checks the companion with the default compiler. Please extend coverage with an apostrophe-containing filename such as Mo's Form.vml and a placeholder-like filename such as @BG@.vml; quoted/backslash replacement values can be checked in a source-generation unit test without creating platform-invalid filenames. Assert that $embed_file, ${err}, and ${event} survive literally in the generated source, and that inserted @BG@ text is not substituted a second time. An automated -new-compiler build of the IDE would also catch the original host-compilation regression, which the current default-compiler companion check alone cannot cover.

@struckchure struckchure Sep 22, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added in b4190a6:

  • test_companion_main_source_escapes_names_and_replaces_placeholders_once: form name Mo's "Form" \ @W@ and file name @BG@.vml; asserts the run_window literal escapes " and \ while passing ' through, that $embed_file("@BG@.vml"), ${err} and ${event} survive literally, that the @BG@/@W@ text coming in through user values is not expanded again, and that no placeholder token remains.
  • test_saved_form_and_generated_main_compile_together now generates and -checks companions for form.vml, Mo's Form.vml and @BG@.vml.
  • test_ide_builds_with_the_new_compiler: full -new-compiler build of ./ide (-check stops before the C stage where the original failure showed up). It skips with a note on a V that doesn't know the flag, which is the case for the 0.5.2 release binaries CI pins. Verified locally that it fails on the parent commit and passes here.

CI now runs v test ui2/ide on every matrix entry. One extra data point: on current V master the new compiler is the default (with a fallback to the cached 0.5.2), so there a plain v test ide against the old template already fails in the C stage with the undeclared err/event.

Cover the generated companion's escaping and single-pass placeholder
replacement: apostrophes, quotes and backslashes in the form name, a
placeholder-like `@BG@.vml` file name, and the literal `$embed_file`,
`${err}` and `${event}` in the emitted source. The compile check now
also generates companions for `Mo's Form.vml` and `@BG@.vml`, and a new
test does a full `-new-compiler` build of the IDE (skipped on a V that
does not know the flag, such as the 0.5.2 release CI uses).

CI now runs the IDE tests on every matrix entry.

Co-Authored-By: Claude <noreply@anthropic.com>
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.

2 participants