Skip to content

Add CDATA LSP support for SceneGraph XML files#1668

Draft
chrisdp wants to merge 20 commits intomasterfrom
feature/cdata
Draft

Add CDATA LSP support for SceneGraph XML files#1668
chrisdp wants to merge 20 commits intomasterfrom
feature/cdata

Conversation

@chrisdp
Copy link
Copy Markdown
Contributor

@chrisdp chrisdp commented Mar 28, 2026

Summary

  • Parses <![CDATA[...]]> script blocks in SceneGraph XML files and registers synthetic BrsFile objects for each block in the program
  • Remaps diagnostics from synthetic BrsFile coordinates back to XML file coordinates so errors appear in the correct location
  • Adds a general CDATA remapping mechanism for all LSP hooks:
    • Code actions (including fix-all): uses _cdataDiagnosticsContext to keep diagnostics in synthetic-file coordinate space during event emission so plugin file-identity comparisons work correctly
    • Completions: remaps cursor position into synthetic file, remaps text edits back to XML coordinates
    • Hover: remaps position in, remaps hover range out
    • Go to definition / Find references: remaps position in, remaps returned locations out
    • Signature help: redirects to synthetic BrsFile
    • Semantic tokens: iterates all CDATA blocks in the XML file, emits per-block, remaps token ranges
    • Document symbols: iterates all CDATA blocks, emits per-block, remaps symbol ranges recursively

Test plan

  • Existing XmlFile.spec.ts tests pass (npm test)
  • Diagnostics (errors/warnings) appear on the correct line inside <![CDATA[...]]> blocks
  • Code actions (including bslint fix-all) show up inside CDATA blocks
  • Completions, hover, go-to-definition, find-references work inside CDATA blocks
  • Semantic token highlighting works inside CDATA blocks

🤖 Generated with Claude Code

@chrisdp
Copy link
Copy Markdown
Contributor Author

chrisdp commented Mar 30, 2026

  • need to add some flag to say should I or should I not be written to the output dir at all 'excludeFromOutput`
  • explore creating file with range offset (or something like that)
  • explore transpliling back into the xml file. Likely from within XmlFile

chrisdp and others added 13 commits March 30, 2026 14:24
…ping

Synthetic BrsFiles for CDATA blocks are now created with leading newlines
and spaces so every position aligns with its parent XML file coordinates.
This removes all toSynthetic/fromSynthetic transforms — LSP handlers pass
positions through unchanged, and results need only a file URI substitution
rather than range remapping.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…e files

- SGScript gains a `transpileSourceNode` property; when set, transpileBody()
  embeds it inline as `<![CDATA[...]]>` rather than writing a uri-based tag
- Program.transpileSyntheticBrsFileToSourceNode() fires beforeFileTranspile
  plugin events and returns a SourceNode whose positions reference the parent
  XML file (offset-padded content ensures no coordinate remapping is needed)
- XmlFile.transpile() pre-transpiles any BrighterScript CDATA blocks via the
  new method and stores the result on the SGScript before the AST transpile
  pass, then cleans up afterward; synthetic files are skipped in the output
- Add tests for transpile output of enums, namespaces, ternary, const, and
  null coalescing inside CDATA blocks
- Add source map tests verifying that generated positions trace back to the
  correct line/col in the parent XML file

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the Program-level `syntheticFileMeta` map with `parentXmlFile`
and `cdataScript` properties directly on `BrsFile`. Diagnostic remapping,
range lookups, and symbol/token iteration now go through the file object
itself rather than a keyed registry.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
SGScript now holds a `cdataTranspile` closure set by Program at file
registration time. transpileBody() calls it lazily, eliminating the
pre-transpile loop and post-transpile cleanup in XmlFile.transpile().

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…events

Re-introduce _cdataDiagnosticsContext (removed in the previous cleanup) and
broaden its scope via emitWithSyntheticFileContext, which wraps every plugin
emit where event.file is a synthetic BrsFile using try/finally. This ensures
plugins calling program.getDiagnostics() from inside any handler — validate,
code actions, document symbols, semantic tokens, transpile — get results where
x.file === event.file holds, fixing the bslint "fix all" pattern for CDATA.

Also adds excludeFromOutput and isSynthetic cleanup from earlier sessions, and
tests covering the context flag for onFileValidate, provideDocumentSymbols,
onGetSemanticTokens, and the full onGetCodeActions "fix all" pattern.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- WorkspaceSymbolProcessor now skips synthetic BrsFiles and instead
  collects their symbols via getXmlFileWorkspaceSymbols(), which iterates
  inlineScriptPkgPaths and remaps each symbol's URI to the parent XML file.
  Previously, symbols from CDATA blocks appeared under the synthetic
  pkg path (e.g. *.cdata-0.script.brs) rather than the XML file.

- Add _pendingSyntheticSrcPaths so that isSynthetic is set on the BrsFile
  before afterFileParse fires. Previously emitWithSyntheticFileContext
  checked isSynthetic after setFile returned, so the _cdataDiagnosticsContext
  guard was always a no-op during the initial parse.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…refactor

- Add JSDoc to all plugin events that may receive a synthetic BrsFile
  (afterFileParse, validate hooks, all LSP events) explaining the new
  CDATA behavior so plugin authors know event.file may be isSynthetic
- Replace fragile two-pass cdataText regex with a single capture-group
  match (/^<!\[CDATA\[([\s\S]*?)\]\]>/) that handles >, trailing
  whitespace, and empty blocks correctly; add five SGParser.spec tests
- Remove _pendingSyntheticSrcPaths side-channel in favor of a configure
  callback param on setFileInternal so isSynthetic is set cleanly before
  afterFileParse fires without program-level mutable state
- Restore blank lines between before/on/after emit calls in getCompletions

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previously synthetic BrsFiles carried two representations of the same
source: padded fileContents (leading newlines/spaces so XML-coordinate
line numbers indexed correctly) and a rawSource passed to the lexer to
avoid spurious Newline tokens from the padding.

Now fileContents stores the raw CDATA text directly. The lexer already
receives startLine/startCharacter offsets so token positions are in XML
coordinate space without any padding needed.

SignatureHelpUtil (the only consumer that needed line-indexed text at
XML-space line numbers) now uses parentXmlFile.fileContents when the
file is synthetic, which has the correct text at every XML-space line.

- Remove rawSource from BrsFile.parse() options
- Remove rawSource from setFileInternal parseOptions
- Remove paddedContent construction in setFileInternal
- Update SignatureHelpUtil to use parentXmlFile.fileContents for synthetic files
- Update fileContents test to assert the new raw-source behavior
- Add cross-file signature help test (CDATA callee, uri-based caller)

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

1 participant