chore: remove xml2st references + fix autocomplete box sizing - #951
Merged
thiagoralves merged 4 commits intoJul 22, 2026
Merged
Conversation
xml2st cleanup (the tool was retired; the in-process ST transpiler replaced it): - Drop stale build/CI/config references (ci-build step name, ci-unit-tests comment, .gitattributes entry, devcontainer binary check). - Rewrite every source/test comment that named xml2st to reference the ST transpiler / STruC++ as appropriate (no behavior change). - Rename the exported PLCopen <addData> namespace openplc.org/xml2st/library-blocks -> openplc.org/library-blocks (nothing reads it back on import; xml2st was its only consumer). plc.xml generation itself is unchanged — it is the PLCopen-conversion / Export-as-XML flow. Autocomplete box sizing: - The graphical (LD/FBD) variable-box autocomplete had a fixed w-36 width, so long suggestions (e.g. struct member refs like some_global_complex.structureVar) were cropped. It now grows to fit content up to max-w-[16rem] and wraps long names (break-all) to multiple lines so the full text is always visible. Byte-identical across the shared frontend surface (compare-surfaces: match, 0 diffs). tsc/lint/prettier clean; changed test suites pass; autocomplete sizing verified live in the browser. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe PR removes obsolete Changesxml2st retirement
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/backend/shared/compile/pipeline.ts`:
- Around line 241-243: Update the documentation for
RunCompilePipelineResult.errors to describe structured diagnostics from the
entire compile pipeline, including ST-transpiler, Arduino compilation, and
strucpp stages, rather than attributing them only to strucpp. Keep the existing
errors field and type unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 842ad1dc-0ccf-47c5-9136-51c0095bf179
📒 Files selected for processing (39)
.devcontainer/post-create.sh.gitattributes.github/workflows/ci-build.yml.github/workflows/ci-unit-tests.ymlsrc/backend/editor/compiler/compiler-module.tssrc/backend/editor/utils/xml-manager.tssrc/backend/shared/compile/__tests__/pipeline-runtime-v3.test.tssrc/backend/shared/compile/__tests__/pipeline.test.tssrc/backend/shared/compile/__tests__/validate-empty-variables.test.tssrc/backend/shared/compile/pipeline.tssrc/backend/shared/compile/steps/generate-defines.tssrc/backend/shared/compile/steps/validate-empty-variables.tssrc/backend/shared/library/__tests__/build-pipeline.test.tssrc/backend/shared/library/__tests__/program-build-helpers.test.tssrc/backend/shared/library/build-pipeline.tssrc/backend/shared/library/program-build-helpers.tssrc/backend/shared/library/program-build-pipeline.tssrc/backend/shared/transpilers/st-transpiler/core/path-tree.tssrc/backend/shared/transpilers/st-transpiler/emit/pou-graphical.tssrc/backend/shared/transpilers/st-transpiler/helpers/block-library.tssrc/backend/shared/transpilers/st-transpiler/walker/ld.tssrc/backend/shared/utils/PLC/__tests__/collect-library-blocks.test.tssrc/backend/shared/utils/PLC/__tests__/split-program-st.test.tssrc/backend/shared/utils/PLC/collect-library-blocks.tssrc/backend/shared/utils/PLC/split-program-st.tssrc/backend/shared/utils/PLC/xml-generator.tssrc/frontend/components/_atoms/graphical-editor/autocomplete/index.tsxsrc/frontend/hooks/use-navigate-to-compile-error.tssrc/frontend/utils/PLC/__tests__/pou-signature-serializer.test.tssrc/frontend/utils/PLC/xml-generator/base-type-tag.tssrc/frontend/utils/PLC/xml-generator/codesys/__tests__/data-type-xml.test.tssrc/frontend/utils/PLC/xml-generator/old-editor/__tests__/data-type-xml.test.tssrc/frontend/utils/PLC/xml-generator/old-editor/__tests__/type-xml.test.tssrc/frontend/utils/__tests__/generate-iec-variables-to-string.test.tssrc/frontend/utils/debugger-session.tssrc/frontend/utils/generate-iec-variables-to-string.tssrc/frontend/utils/iec-types-registry.tssrc/middleware/shared/ports/compiler-port.tssrc/middleware/shared/utils/library/compose-runtime-v4-bundle.ts
💤 Files with no reviewable changes (2)
- .devcontainer/post-create.sh
- .gitattributes
Comment on lines
+241
to
243
| /** Structured strucpp diagnostics from this run. Carries | ||
| * the per-error events the renderer's navigation keys off. */ | ||
| errors?: StructuredCompileError[] |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Document all pipeline error sources, not only strucpp diagnostics.
RunCompilePipelineResult.errors can receive ST-transpiler errors and Arduino compilation errors as well as strucpp errors. Calling the field “Structured strucpp diagnostics” makes the public result contract misleading; describe it as structured compile-pipeline diagnostics or split it by stage.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/shared/compile/pipeline.ts` around lines 241 - 243, Update the
documentation for RunCompilePipelineResult.errors to describe structured
diagnostics from the entire compile pipeline, including ST-transpiler, Arduino
compilation, and strucpp stages, rather than attributing them only to strucpp.
Keep the existing errors field and type unchanged.
…me never wraps When the suggestion list overflows and the vertical scrollbar appears, it stole horizontal space the content-based auto-size didn't reserve, wrapping the last character of the widest name onto a new line. Add `scrollbar-gutter: stable` to the scroll container so the reserved gutter is included in the auto-size. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Safari-specific rendering of the LD/FBD variable-box dropdown: - `w-max` pins the popover to the widest item's full single-line width (clamped by min/max). Safari's shrink-to-fit otherwise collapsed the width toward min-content when items use `break-all`, wrapping names unnecessarily. - `shrink-0` on the item rows, the "Add variable"/new-block rows, and the variables container. Safari vertically shrinks flex-column children, so a wrapped name's second line overflowed its box and rendered on top of the "Add variable" button below. Keeping full height pushes siblings down instead. Chrome was unaffected (sizes to max-content and doesn't shrink these rows); verified no Chrome regression. Safari behavior to be confirmed by the reporter. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ropdown Root cause of the Safari-only cropping: `word-break: break-all` collapses WebKit's max-content computation, so the auto-sized dropdown came out too narrow and wrapped names (even short ones), and the wrapped line overlapped "Add variable". `overflow-wrap: break-word` (Tailwind `break-words`) does NOT collapse max-content — it still breaks over-long names — so the popover sizes to the full name (clamped to [9rem, 16rem]) in both Chrome and Safari. Dropped the now-unneeded `w-max` (its interaction with min-width under WebKit was part of the problem); Radix's `min-width: max-content` wrapper drives the sizing. `shrink-0` rows retained so a genuinely long name that wraps still pushes "Add variable" down instead of overlapping it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Byte-identical companion PRs (shared
frontend/backend/shared/middleware/sharedsurface —compare-surfaces.pymatch, 0 diffs). Two changes:1. Remove xml2st references
The xml2st tool was retired (replaced by the in-process ST transpiler). This removes the leftover references so the code is xml2st-free:
ci-build.ymlstep name,ci-unit-tests.ymlcomment,.gitattributesentry,.devcontainer/post-create.shbinary check. (Editor-local untracked artifacts — the 31 MBresources/bin/.../xml2st/tree, its backup, and the.binary-metadata.jsonkey — were also removed locally; they are not tracked anddownload-binaries.tsno longer fetches xml2st.)openplc.org/xml2st/library-blocks→openplc.org/library-blocksfor the PLCopen<addData>block-signatures payload. Nothing reads it back on import (xml2st was the sole consumer), so this is a safe rename; the assertion test was updated.plc.xmlgeneration itself is untouched — it remains the PLCopen-conversion / Export-as-XML flow.Not touched: historical docs (
docs/strucpp-migration/*,docs/transpiler/*,CLAUDE.md's "xml2st retired" note, webdocs/*) that document the xml2st→strucpp migration — removing xml2st from those would erase the record. Flag if you want those gutted too.2. Autocomplete box sizing
The graphical (LD/FBD) variable-box autocomplete was fixed at
w-36, cropping long suggestions (e.g.some_global_complex.structureVar). It now grows to fit content up tomax-w-[16rem]and wraps long names (break-all) to multiple lines, so the full text is always visible.Verification
tsc 0 errors, ESLint clean, Prettier clean; changed test suites pass (editor 235, web 228); autocomplete sizing verified live in the browser on
dev:local.🤖 Generated with Claude Code
Summary by CodeRabbit
matiecbinary.