fix(AppShell): stop expression-mode script params reverting to simple mode - #2171
Open
alexwarren wants to merge 1 commit into
Open
fix(AppShell): stop expression-mode script params reverting to simple mode#2171alexwarren wants to merge 1 commit into
alexwarren wants to merge 1 commit into
Conversation
… mode isSimpleValue()'s default case classified any value that merely started and ended with a `"` as a simple quoted-string literal, so a compound expression like `"a" + b + "c"` (e.g. Print's message field holding `"It weighs " + object.weight + " grams."`) was misdetected as simple. Any mutate()-triggering edit anywhere in the script tree calls refresh(), which unconditionally clears expressionOverrides, so the mode dropdown then re-derived "simple" from this flawed check and silently flipped back to message mode, mangling the display via toSimpleDisplay's slice(1, -1). The underlying saved script text was never actually corrupted - only the display was - but this made it look like the typed expression had been lost. Tightened the check to require the whole value be a single valid string literal (no unescaped quotes in the middle).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
isSimpleValue()inScriptEditor.svelteclassified any script-parameter value as a "simple" quoted literal whenever it merely started and ended with", so a compound expression like"It weighs " + object.weight + " grams."(a Print message set to expression mode) was misdetected as simple.mutate()-triggering edit anywhere in the script tree callsrefresh(), which unconditionally clearsexpressionOverrides, the mode dropdown re-derives "simple" from this flawed check on the very next render — silently flipping the field back to "message" mode and mangling the displayed text viatoSimpleDisplay()'sslice(1, -1)(which assumes the first/last characters are the literal's own quotes).GetScriptCode/raw XML round-tripped the real expression the whole time — but the display bug made it look like the typed expression, and the save itself, had silently failed. Confirmed via Playwright that autosave does complete ("Saved" chip) even while this bug was live; only the visual state was wrong.", only non-quote/escaped-quote characters, then a closing") rather than just checking the first/last characters.expressionField's template controls (e.g. "object has flag"), and any othertextbox/dropdown/file-style script parameter — all route through the same sharedisSimpleValue(). Theobjects/boolean/numbercases already use stricter, unaffected checks.Test plan
npm run check(svelte-check) — cleannpm run lint(eslint) — cleantests/e2e/verify-appshell-print-expression-mode.mjs, verified it fails against the pre-fix code and passes against the fixnode tests/e2e/find-affected-tests.mjsflagged 18 scripts touchingScriptEditor.svelte; ran all 18 locally against the dev server — all pass🤖 Generated with Claude Code