Dev/dialog editor#100
Merged
Merged
Conversation
…ranch refuse)
Wire addBranch/addElse/removeBranch ops into DialogGraph actions (Tier 3c,
gated by structEditable + rebuild). Inspector gains three controls inside the
{#if state.branches} block only:
- Per-branch remove button (X) in the branch head, disabled-with-tooltip when
b.opaque.length > 0 (side-effect refuse: cannot splice opaque stmts safely).
- After the branch list: an inline condition input + "+ if" button (disabled
until a non-empty condition is typed) that calls addBranch then clears.
- A "+ else" button shown only when there is exactly one if-branch and no else.
The new-branch condition input uses a writable store instead of $state: the
Svelte 5 compiler misidentifies $state() as a store subscription when a local
binding named `state` (the node prop) is in scope, leaving the variable
non-reactive and breaking the disabled check and programmatic clear.
…ten unverified inspector comment
In branchStructureOps ADD-if path, the insert offset was computed as
Math.max over ALL original branches' stmtRange.end. When the last branch
was being removed in the same save, its end was used as the anchor for the
new branch splice, technically overlapping the removal. Filter to originals
whose stmtRange.start is in editedIfStarts (the survivor set the REMOVE pass
already built), anchoring at the max surviving end. Fallback when all
originals are removed: min original stmtRange.start (inside the procedure
body, safe under right-to-left application).
New test pins the edge: two if branches, remove the last and add a new
sibling if in one save - asserts the new branch lands after the surviving
branch (not at the deleted branch's end), the survivor is byte-exact, the
output re-parses bundleFaithful, and verifySSLEditApplied is ok.
Inspector.svelte: tried $state("") locally to verify the compiler warning -
build produced two warnings (non_reactive_update + state_referenced_locally,
both from the state prop name shadowing the $state rune). Kept writable;
updated the comment to state the observed build-warning behavior only, without
asserting the compiler-internals mechanism.
… an entry The entry-call addition filter excluded every renamed node on the assumption its call already existed, but a node that gains entry status for the first time while being renamed has no prior call to rewrite - so it was silently left unwired. Identify a renamed node in the original entry set by its old id (renamedFrom) instead of dropping all renamed nodes.
applySplices applies ops right-to-left, which is only correct when their spans are disjoint; an overlap silently corrupted the output. Add a guard that throws on an overlapping or start>end op, and cover the splice contract (overlap, adjacent, zero-width-inside-range, reversed, single-char, empty) directly - the core every save path depends on previously had three tests.
Reachability: self-looping entry stays reachable, self-looping non-entry island is orphan, single-state graph, empty-root classification. Layout: single node with no edges, and a back-edge cycle terminates with finite distinct positions.
…on, relocate VerifyResult - Remove targetLabel: exported but referenced nowhere. - Drop layoutFlow's direction parameter and its dead DOWN path; the sole caller always uses the default RIGHT, matching the module's left-to-right design. - Move VerifyResult to dialog-splice (the shared core both editors import) so dialog-ssl-edit no longer depends on dialog-d-edit for a format-neutral type.
…mment - Replace three identical inline @N-id-parsing lambdas (and a fourth in the new-node path) with one module-level atMsgId helper. - The badge-vocabulary comment claimed computed/random/side-effect/virtual-sink had no producer; all are produced now (stateBadges/choiceBadges) - correct it. - Document why callTransitions dedups by target name and what fixing the duplicate-call rename gap would require (model-layer, not just the parser).
…view polish - Tree view: a bundle (if/else) node now renders each branch with its own 'shown when ... / otherwise' header, NPC line, and replies, instead of flattening to the if-branch line plus all options. The else branch's line was previously dropped. conversation-tree builds the branch structure; each choice is expanded once (via its branch), never double-marked shown. - Tree: render the derived marker through the shared Badge component (drop the hand-rolled badge span + duplicated CSS). - Inspector: compute branchChoices once per branch; rename the 'logic' drawer to 'side effects' (it holds source-only side-effect statements). - Graph legend: dash the 'continue' key so the dashed-edge encoding is documented.
A node may `call X;` more than once (e.g. one call per if-branch). The parser deduped call transitions by target name, so the model carried only the first call site - rename rewrote one site and left the others as dangling `call X;` (sslc-rejected), and delete spliced out only the first inbound call. Record one call transition per statement (callTargets stays deduped to one graph edge / one call-choice per unique target). Replace the single-cardinality callStmtRange/callTargetRange/callTopLevel on DialogChoice with a callSites list so rename rewrites every target token and delete removes every top-level site. 49 node-to-node duplicate-call sites exist in the test corpus.
…imap The inspector (top-right Panel) capped at calc(100vh - 96px), but the svelte-flow minimap sits bottom-right in the same column. A tall node's inspector grew down into it (measured 112px overlap across the minimap's full width at a 640px-tall viewport). Reserve the minimap band (~152px widget + 15px margin) plus a gap in the cap so the inspector bottom stays ~12px above the minimap top at any viewport height.
…rness Applying the render-STATE sweep across the graph view surfaced two more layout collisions between individually-correct panels: - The top-center legend was overwritten by the variable-width top-left toolbar (a wide toolbar reaches center), hiding the Source/Issues buttons. Fold the legend into the toolbar as one flex-wrap container capped to leave the top-right inspector's column clear - it wraps downward, never into a sibling. - The bottom-left D source panel covered the fixed bottom-left zoom controls. Lift it above them (not right, which would push it into the centered Issues). Add a render-STATE guard to the production-path harness: with the selection inspector + Source + Issues open at once, assert no two overlays' bounding boxes intersect - the busy state a single default frame never exercises.
… its body A node DELETE removed the procedure body and retargeted inbound options but left an orphaned 'procedure <name>;' forward declaration in the saved SSL. Capture the full forward-decl statement span in the parser (distinct from the name-token span used by rename) and splice it out on delete. Test asserts zero dangling symbols after delete.
…de the canvas The inspector, D-source preview, and issues panels floated over the graph as absolutely-positioned overlays. At the narrow width the webview actually runs at (it opens beside the text editor, sharing the VS Code window, ~270-540px) they collided with each other and the canvas chrome (minimap, zoom controls) - the prior collision guard only tested at 1100px and never saw it. Dock all three auxiliary panels into one shared right rail beside the canvas, for both graph and tree view modes. This removes the whole floating-panel collision class by construction rather than repositioning per width, and unifies the two modes' auxiliary-panel handling (the tree mode previously duplicated it as .tovl overlays). Also: hide the minimap on a narrow canvas (it otherwise sits atop the zoom controls, and is not useful that small), and cap the top-left toolbar to the canvas width so it wraps down within the canvas instead of overflowing into the rail. The tree toolbar and the legend now wrap too, so a narrow canvas can no longer clip the legend or strand the Graph/Tree toggle. The render harness now drives the collision guard at 300/520/900px (the real embedded widths, narrow and wide) instead of a single roomy 1100px frame.
The dialog editor is a WebviewPanel opened beside the text document, not a CustomTextEditor, so VS Code's own Ctrl+S does not reach the underlying .d/.ssl from the webview - the shortcut was a dead key and saving required the toolbar Save button. Wire a window keydown to the same host save the button uses, for Ctrl+S (and Cmd+S on macOS), guarded so it no-ops with no host. The chord predicate is extracted to a pure keyboard.ts and unit-tested (the cross-platform Cmd case is the one worth guarding), matching how the webview's other pure decisions live in app-messages.ts; the window binding stays in the component.
…ntry be unset
Two friction points a modder hit:
- Deleting a state silently redirected every transition pointing at it to EXIT.
Now a delete that would redirect inbound transitions pops a confirm naming the
count ("N transitions pointing here will be redirected to EXIT"); with no inbound
refs it deletes straight through. countInboundGotos is a pure helper in
dialog-edit-ops (unit-tested), and the render harness drives the confirm/cancel
flow end to end.
- The SSL entry checkbox was one-way: marking a node as a dialog entry immediately
disabled the box (the removable check read the on-disk entry calls, which don't
yet include the pending toggle), so it couldn't be un-marked until save+re-parse.
A node made an entry in-session can now be un-marked freely (the save adds no
call, so nothing is orphaned); a genuinely non-removable original entry (wired by
force_dialog_start or a conditional call) stays disabled and now carries a tooltip
explaining why and pointing to the .ssl source.
…source tree The binary-editor render harnesses wrote their shot-*.png outputs into the harness dir itself, cluttering the working tree whenever a harness runs. Route them through a shared shotPath() helper (out-dir.ts) that resolves the repo-level tmp/ dir, matching the dialog harness (client/src/dialog-editor/test/harness/ render.mts). All 16 screenshot-writing harness files updated; verified the harness typecheck passes and a sample run lands its PNGs in tmp/.
…path
svelte-flow's built-in delete key (Backspace/Delete on a selected node) removed
the flow node directly, bypassing the model edit ops: no confirm, inbound GOTOs
left dangling, the Issues panel unaware, and nothing undoable - a live review hit
exactly this and could silently corrupt a dialog. The confirm added earlier only
covered the inspector button and tree menu; the keyboard was an unguarded cohort.
Disable svelte-flow's built-in delete (deleteKey={null}) and handle Delete/
Backspace ourselves, routing every delete path through one requestDeleteState gate
(reject non-deletable, confirm the EXIT redirect, then delete). The key handler is
skipped while typing in a field or while the confirm modal is open. Harness now
asserts Backspace on a referenced node pops the confirm instead of silently
deleting.
Also gitignore .playwright-mcp/ (Playwright MCP snapshot output written to the repo
root during live UI review).
…sation tree The Tree view had two accessibility gaps a live review caught: each state row was a focusable div wrapping a focusable caret button (two inconsistent tab stops per row), and the arrow keys did nothing. Adopt the WAI-ARIA tree pattern: the container is role=tree, each state row is a role=treeitem with aria-level, aria-selected and aria-expanded; the expand caret is taken out of the tab order (tabindex=-1, aria-hidden) so each row is a single tab stop; and a roving tabindex plus ArrowUp/Down (move between rows) and ArrowRight/Left (expand/collapse) give real keyboard navigation. Harness asserts the roles, the absence of tabbable carets, and that ArrowDown roves focus.
…logSourceEdit The two existing tests only exercised weidu-d, leaving the allocateNodeIds / allocateOptionIds branch - the exact logic the extraction exists to protect - with zero executed assertions. Add a fallout-ssl test that adds a new node and a new option with un-allocated literal text, drives the real allocation, and asserts both the spliced source and the merged messages map. Also assert result.messages on the existing D tests, which previously checked only newText.
…ovider Replace the command-owned WebviewPanel with a CustomTextEditorProvider bound to the source document, so VS Code supplies native dirty tracking, undo, and save. resolveCustomTextEditor wires the webview HTML and message routing; on "ready" it parses and posts the model; on an edit action it splices into the live document via a single full-range WorkspaceEdit (one native undo step) and write-throughs message text to .tra/.msg on a short debounce plus on native save. An EchoGuard suppresses re-projecting the graph in response to the provider's own WorkspaceEdit, while still re-projecting external text edits. Per-panel session state (guard, latest model, debounce timer, pending verify) replaces the old module-level mutable panel/docUri/refreshTimer. The webview still posts a legacy whole-model "save" message alongside the target "edit" message; both route through the same applyEdit path until the webview switches to per-action edit emission and drops the Save button. The command becomes a thin vscode.openWith wrapper. registerDialogEditor keeps its existing signature so extension.ts is unchanged.
…t mark, drop latent verify
…tton and hand-wired Ctrl+S
…t divergence on rejected edit
… shared text gate
A just-added SSL node's NPC line was locked ("this line has no plain @n
message id") the moment the webview adopted the reparse: the per-call-site
`isNew` proxy used `isPendingState`, which flips false once the node gains a
`procRange` from the fresh parse, even though its Reply is still unwritten.
Introduce `textEditability`, the single decision both views (inspector fields
and inline tree edits) consume, returning the lock and its reason together so
neither re-assembles `textFieldLocked`'s inputs at the call site. It resolves
the authorable proxy by owner: an option keys on `isPendingChoice`, a node's
NPC line on the new `npcLineAuthorable` (pending OR a faithful reply-less node
whose Reply the save path allocates).
Back the reply-less case with a `replyless` flag on DialogState, set at SSL
parse under the exact `replyOps` precondition (faithful, no source Reply, and a
node-level insertAnchor). The writer's orphan-id set now reads the same flag, so
the UI gate and the save path agree by construction - a bare `text === ""` check
would mint ids for anchorless TSSL nodes the writer can never splice.
…y can't mutate the original A duplicated node cloned its source line's bare @n ref verbatim, so the copy pointed at the SAME .msg/.tra entry as the original. Editing the copy's line (writeText keyed on that shared id) then silently rewrote the ORIGINAL's line too - a copy that corrupts its source on the first edit. duplicateState now detaches each resolvable @n ref to its literal, so the copy is a new-node literal the save allocator mints a FRESH id for and the copy owns an independent string. An unresolved ref (its .msg never loaded) has nothing to detach and stays a ref - harmless, since editing an unresolved line is already locked. Textless "continue" options and computed/random text are left untouched. Applies to both families (SSL and D both store shared strings as @n refs).
…write an invisible orphan +State on an existing SSL dialogue splices `procedure NodeXXX begin end`, but the projection only kept an unreachable procedure that already had a reply or option - so the empty node was written to disk yet dropped on reparse, vanishing from the tree while leaving a husk in the source (BUG A). An empty `NodeNNN`-named, non-hook procedure is a dialog node in progress; keep it visible. Its NPC line is authorable through the `replyless` gate, so the node shows AND can be filled in immediately, and it survives every reparse because it genuinely round-trips (an empty body is faithful). Existing scaffolded empty stub nodes now show too - a net improvement for a tree editor. Reserved sinks (Node998 combat / Node999 end) stay excluded from the orphan path: they are transition targets, not nodes. A renamed empty node (not NodeNNN) still needs wiring to appear; the general pending-node layer will cover that.
…silently vanish Editing a D/TD NPC line backed by an @n whose .tra never loaded looked like it worked - the tree updated, the unresolved count dropped, the tab read "saved" - but the edit only touched the in-memory message map; the server's writeMessages found no .tra target and returned without throwing, so nothing reached disk and no error surfaced (BUG E). Unify the text gate: a bare @n line is editable only when its .msg/.tra entry actually loaded, for BOTH families - an unresolved ref has nowhere to write, so lock it and fail visibly (a disabled field with a "point translation.directory at the .tra" reason) instead of accepting an edit that drops on save. SSL already did this; the D family short-circuited to always-editable. Literals stay editable for D (persisted via the .d splice) and locked for SSL, unchanged. The reason is now family-aware (.msg for SSL, .tra for D).
…s to the gate The silent "what I see isn't what's saved" bug class hid because no gated test drove the full production edit pipeline and asserted the on-disk bytes against the intended view. Add two end-to-end guards through the real parse -> edit op -> computeDialogSourceEdit -> writer -> reparse path: - +State on an existing dialogue creates a node that survives the reparse and can then be filled in and persisted (BUG A + the replyless NPC-line authoring loop). - Duplicating a node and editing the copy leaves the original's on-disk string untouched, the copy owning a fresh independent @n (BUG C, end-to-end on disk). Both run in the server unit gate; each fails against the pre-fix code (Node002 dropped on reparse; @200 rewritten by the copy's edit).
…-line .msg/.tra edit The inspector's NPC and option fields are textareas (they wrap/autosize long lines for reading), so pressing Enter inserted a newline that the write-through persisted - the closing delimiter ended up on its own line and the stored string carried a trailing newline (BUG D). The inline tree editor, a single-line input, never had this. Two guards: writeText folds every CR/LF/CRLF to one space on write, so no newline can ever reach a .msg/.tra entry regardless of the input widget (a paste included) - newlines only, not a trim, since writeText runs on each keystroke and trimming would strip a space the user is mid-typing. And Enter now commits (blur) on both fields instead of inserting a newline, matching the tree's inline input; Shift+Enter stays a browser escape hatch that the fold still neutralizes.
… legend The "Del delete" legend read as applying to whatever was selected, but the key handler only ever deleted the selected STATE - selecting an option and pressing Del silently removed its parent node instead (BUG G). Options could only be removed via the row `x`. Del now removes the selected option through the same gate its row `x` uses (a structurally-editable flat node, not a conditional SSL option that sits in its own `if` the writer won't rewrite); a locked or branch option is a no-op, never a surprise node delete. With no option selected, Del deletes the selected state as before. This unifies the delete gestures so the keyboard, the row `x`/`-`, and the context menu are one path, and makes the legend honest.
…ad of dropping the delete Deleting an NMessage/GMessage/BMessage terminal option was a silent no-op: a terminal carries a stmtRange but no callRange, so it was invisible to nodeOps' callRange-based remove pass and the option stayed on disk. Since a new option defaults to EXIT -> NMessage, "add an option then delete it" hit this exactly - the added option could not be removed once saved. nodeOps already special-cased the terminal->node flip for the same callRange-blindness; add the missing terminal-removal pass alongside it, splicing out any flat source terminal whose id is gone from the edited node (a surviving or retargeted terminal is skipped; a conditional terminal is left to Tier 3). Shared by the SSL and TSSL writers, so both families gain it. The line-aware splice leaves no blank-line residue.
…t-gate unification The E-fix removed textFieldLocked's D short-circuit, so the parenthetical explaining why isPendingChoice gates on sourceRange was stale. A D option mis-read as pending is now treated as isNew (always editable), which would let an unresolved-@tra option slip past the very lock the D gate just added - restate the rationale in those terms.
… usability testing The @N-unresolved help (webview banner + the locked-field tooltip) and docs/settings.md all told the author to set `translation.directory` in .bgforge.yml, but the reader (settings.ts) only reads it nested under a top-level `mls:` block - so a hand-written top-level `translation:` silently never resolves and every @n stays raw. Point all three at `mls.translation.directory` and show the nested YAML the code actually parses (the auto-created config already used this shape; only the guidance was wrong). Separately, a node forced read-only by a trailing non-dialog statement (e.g. a `set_map_var`) was told it "nests if/else conditions" when it has none: the structured tier is entered by a nested if/else OR a preserved non-dialog statement, so the reason now names both instead of if/else alone.
…, not just the first A WeiDU D `SAY @A = @b = @c` state kept all its lines in the model (sayTexts) but every view rendered only the first, so a 9-line monologue showed one line and the rest were invisible and uneditable. Surface them everywhere: the tree renders the continuation lines as muted, indented rows after the primary line (a monologue reads as a sequence); the inspector shows each as its own "NPC line (cont. N)" field; the graph card - the compact overview - shows the first line plus a "+N more" chip. ConvState gains `sayLines` (the resolved continuation lines) and `sayLineEditability` gates each field with the same @N-resolvability / literal rules as any other line. Editing any line persists: an @n line writes its .msg/.tra entry (the .d keeps `@a=@b=@c`); a literal line updates sayTexts and the writer re-joins it. That exposed a silent-drop the display made reachable - stateUnchanged and the D writer's SAY-change check both keyed on line 0 (`text`) only, so a continuation-only edit read as unchanged and never spliced. Both now compare the whole sayTexts, so a literal continuation edit reaches the source. Live-verified: editing continuation line 2 rewrote only that alternate on disk, lines 1 and 3 byte-preserved.
…isn't in source yet An item you add lives in the webview's optimistic model before the reparse adopts it into the source parse; an empty option is deferred until its text commits. Until then it rendered identically to a committed item, so there was no cue that it was unsaved (the "no cue" usability note). Surface it: ConvState/ConvReply gain a `pending` flag (from the existing isPendingState/isPendingChoice predicates). The tree marks a pending node/option with a 2px amber left-accent and an "unsaved draft" tooltip; the graph card gets a dashed amber border and an "unsaved" badge (badge text, not colour alone, carries the meaning). Committed items are unmarked. Live-verified in code-server: an added-then-abandoned empty option shows the amber draft accent while its committed siblings do not. The card cue follows the same isPendingState predicate (svelte-check clean); a pending NODE commits within the ~250ms emit/adopt window, so its badge is transient by nature.
New WeiDU D states default to StateNNN (next free, zero-padded) instead of new_state/new_state_1, mirroring the SSL NodeNNN scheme; duplicates use the same allocator rather than <id>_copy. The auto-name toggle tooltip and the add-node comment now say StateXXX for the D family.
… cleanly The add-node writer anchored the new id just before the append list's close token, so a multi-line variadic append with a trailing comma after its last element (what a formatter emits) got a second comma - an empty call argument that is a hard TypeScript syntax error, breaking the parser for the whole file. Anchor after the last element instead, leaving any trailing comma valid. Adds a regression test over the real multi-line variadic-with-trailing-comma shape.
Three write-back bugs that only surface on real authored dialogs, each found by the new corpus round-trip tests: - D: rename/delete did not rewrite GOTO references inside EXTEND/CHAIN/INTERJECT constructs, leaving a dangling jump (a WeiDU compile error). Mirror the model retarget into those derived spans (rename rewrites the label; delete flips the GOTO clause to EXIT). - SSL: deleting a node whose inbound option carried a computed message id left the option pointing at the deleted node. Preserve the computed id text and emit a terminal NMessage instead of bailing. - SSL/TSSL: renaming a node referenced from inside an if/else bundle branch double-spliced the reference span (the per-node bundle engine already rewrites it), crashing applySplices. Skip the standalone rewrite for bundle nodes. Adds a shared corpus round-trip harness that runs the full edit battery (retarget, add/remove/reorder option, add/remove/rename node) through the real apply*DialogEdits over the external corpora for all four families, asserting targeted reachability-robust invariants.
… divider line Adds a border-bottom under the beta/feedback row (matching the toolbar's other #2b303a divider) so the notice reads as a distinct band above the button row.
… sleep render-map-jump-deep raced a fixed waitForTimeout(300) after clicking the SID jump chip, then read the object detail's SID field. A deep jump is two async round-trips - locateEntry fetches the full object list to resolve a selection past the bounded window, then the detail pane fetches and renders the landed object's fields - so at 300ms the detail pane was still empty and the read returned null, failing the assertion even though the jump had landed on the correct object (verified by screenshot). Replace the sleep with a waitForFunction poll on the SID field rendering; the assertions still report the concrete value on a real regression rather than a poll timeout.
The binary-editor and dialog-editor Playwright harnesses assert through PASS/FAIL gates (driver-util, clip-gate, per-driver failed counters, page-error and CSP checks) but nothing ran them, so they caught regressions only when a human remembered to. Wire them into CI so they guard every push. - scripts/test-harness.sh: build the binary core + both app.html bundles, then run every assertion-bearing driver (all 20), failing on any non-zero exit. - .github/workflows/harness.yml: a SEPARATE job (not the main Build gate) so the Chromium download and browser runs add no wall-clock to every other check. It installs Chromium, builds grammars (the dialog edit drivers parse .d through the weidu-d WASM), resets the external fixture repos, then runs the suite. - playwright becomes a pinned devDep (exact 1.61.1): tsx/ESM won't resolve a global install (NODE_PATH is ignored for bare ESM specifiers), so it must live in workspace node_modules. pnpm's build-script gate skips its browser download, so a plain install stays lean; the harness paths install Chromium explicitly. - With playwright resolvable in CI, the harnesses are typechecked too: the dialog-editor harness via "Typecheck client" (it sits under client/src), the binary-editor harness via its own DOM tsconfig as a new test.sh step. - Docs updated for the devDep + the new CI suite (AGENTS.md, harness READMEs).
…ew drivers
Every render/drive harness driver raced fixed waitForTimeout(N) sleeps that,
under CI load, could read the DOM before the awaited async work (a fetch, a
re-render, a debounced round-trip) landed - the same flake the deep-jump driver
hit. Convert each to a condition wait:
- sleeps before a Playwright locator action: deleted (locators auto-wait);
- sleeps before a raw page.evaluate read that feeds an assertion: replaced with
page.waitForFunction polling the specific condition the read expects, with
.catch(() => undefined) so a real regression fails on the concrete assertion
rather than a poll timeout;
- a few bounded waits kept + commented where no crisp condition exists: asserting
an ABSENCE (no husk spliced, no echo-loop emits), a same-model re-post/adopt
that yields no distinct DOM signal, or a negative invariant on a disabled control.
Also fix a latent timeout bug: page.waitForFunction(fn, { timeout }) is the 2-arg
form where the options object binds to the callback ARG, so the poll silently used
Playwright's 30s default instead of the intended 5s (verified against
playwright-core 1.61.1). Passed undefined as the arg at all 51 sites.
The suite runs deterministically green across repeated runs and ~2x faster (polls
resolve as soon as the condition holds; sleeps always waited in full).
…ushed past EOF
An empty CRE section's offset is preserved verbatim from the header (not
recomputed) so an unedited fixture round-trips byte-exact - across the corpus it
almost always already equals the computed in-place position, but a fixture can
store a different in-bounds value there (imoen.cre) and preserving it keeps that
file exact. A size-shrinking edit - removing a spell-memorization owner and its
slice on x#beador.cre drops 88 bytes - could leave that preserved offset pointing
PAST the new EOF (effects empty, offset 0x558 vs a 1360-byte file), and the parser
rejects `offset > size` even for a zero-length section, so the op produced bytes
that would not reparse ("Missing CRE group: CRE Header").
Fix at the single serialization choke point: where a preserved empty-section
offset now exceeds the new total size, fall back to the computed in-bounds
position; a still-valid offset is left untouched, so unedited round-trips (and the
imoen.cre outlier) stay byte-exact. Guarded by the existing fixture-driven
cre-entity-ops regression over x#beador.cre.
…onent flags These three WeiDU component flags were missing from the grammar, so any component using them (e.g. BG1NPC's bg1npc.tp2) failed to parse and the formatter rejected the whole file. Add the flag rules (message required, matching the sibling *_component / *_predicate flags), route the new predicate flag through the same OR/AND condition splitter as REQUIRE_PREDICATE in the formatter, and add tree-sitter highlight + corpus coverage. TextMate highlighting and completion/hover data for these flags are not yet wired up; tracked as follow-up.
…e format sweep BG1NPC (added to the IE corpus as a dialog test corpus) is heavily templated. Two classes of its files are not valid standalone WeiDU and so cannot go through the formatter's parse+idempotency check: - 184 BAF files use WeiDU %variable% placeholders at trigger/action position (e.g. IF %BGT_VAR%), substituted at install time. weidu rejects them standalone too; they are the same class as the existing %variable% exclusions. - 3 TPA files carry a stray extra END (unbalanced BEGIN/END). weidu tolerates the trailing END, our stricter grammar does not. The remaining BG1NPC parse failures (valid .d and .tpa constructs weidu accepts) are real grammar gaps, fixed separately.
Two valid WeiDU constructs BG1NPC uses that the grammar rejected: - EXTEND_TOP/EXTEND_BOTTOM accept an optional USING ~tra~... clause after EVALUATE_BUFFER (as COMPILE already did); without it the tra path was mis-parsed and reported a spurious MISSING '='. - REMOVE_CRE_ITEMS (plural, no argument) strips every item from a CRE, distinct from the singular REMOVE_CRE_ITEM ~resref~ that was already supported. weidu accepts both (cross-checked with --parse-check); add corpus coverage for each.
CHAIN3 is WeiDU's improved chain keyword; it is syntactically identical to CHAIN (same optional IF/WEIGHT/THEN header, file, label, chain text, epilogue) but was not accepted by the grammar, so all 26 BG1NPC dialogs that use it failed to parse. Accept it as an alternative chain keyword; the formatter reads the actual keyword token, so CHAIN3 round-trips unchanged (verified it is not normalized to CHAIN). Add corpus coverage.
… constructs Valid WeiDU D constructs (weidu --parse-check accepts them) that the grammar rejected, so BG1NPC dialogs using them failed to format: - %var%-interpolated names: %tutu_var%KELDDA etc. - a name assembled at compile time from a %var% substitution abutting literal text. Modeled as interpolated_name using token.immediate so pieces only combine with no intervening whitespace; added to the filename position. - Hyphenated state labels (Quayle_Shar-Teel_1): allow '-' in state_label_alnum. - ALTER_TRANS change with a bareword field (ACTION ~..~), not just the quoted "ACTION" form. - A trailing IF/UNLESS guard after an ADD_TRANS_ACTION action string (WeiDU accepts the guard on either side). Full IE .d corpus reparses with zero regressions; formatter round-trips all four unchanged. Corpus coverage added for each.
weidu --parse-check rejects this file; it uses an unresolved %kivan17% macro as standalone CHAIN content. Same class as the malformed .tpa - weidu tolerates neighbouring valid constructs but the file itself is not parseable standalone, so keep it out of the format sweep.
…empotency bug x#faint.d is valid (weidu accepts it; grammar parses it) but the D formatter is non-idempotent on it: the source has a tilde-string abutting THEN with no space (~..NOTVALID)~THEN), and normalizeTransitionText emits ~..~THEN on the first pass but ~..~ THEN on the second. This is a pre-existing formatter bug, masked until the format sweep got past the parse failures fixed earlier in this branch; it is orthogonal to the grammar work and needs a careful fix (add the space only for ~/" - delimited strings, never % - interpolated names must stay abutted). Parked here and tracked separately; unpark once the formatter is fixed.
A transition trigger written with no space before THEN (~..NOTVALID)~THEN, valid WeiDU) formatted to ~..~THEN on the first pass but ~..~ THEN once the broken line was reparsed, so the formatter was not idempotent and the external format sweep failed on BG1NPC's x#faint.d. normalizeTransitionText now inserts a separating space at a ~/"-delimited string <-> word-character boundary (either direction), so the trigger and the following keyword are always spaced. %-delimited strings are deliberately excluded: %var%-interpolated names (%tutu_var%KELDDA, TAZOK%eet_var%) abut literal text on purpose and must not be split. Unparks x#faint.d (previously excluded for this bug). Full IE format sweep is green with it back in; added a CLI idempotence regression test.
| const configPath = path.join(this.workspaceRoot, ".bgforge.yml"); | ||
| if (fs.existsSync(configPath)) return; | ||
| try { | ||
| fs.writeFileSync(configPath, yaml.stringify({ mls: { translation: { directory: relDir } } })); |
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
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.
Dialog editor and assorted fixes.