Skip to content

Preserve SEU/5250 control bytes when saving source members - #3516

Draft
Balrocj wants to merge 2 commits into
codefori:masterfrom
Balrocj:Source-members-with-control-bytes-in-column-6
Draft

Balrocj wants to merge 2 commits into
codefori:masterfrom
Balrocj:Source-members-with-control-bytes-in-column-6

Conversation

@Balrocj

@Balrocj Balrocj commented Nov 5, 2026

Copy link
Copy Markdown

Fixes source member saves when SEU/5250 control bytes are present in column 6.

Issue: #3514

Some IBM i source members include legacy control/display bytes in fixed-format positions. These bytes are not normal text and should not be rewritten or stripped during the save flow. Previously, saving a member could rewrite those bytes as \Z or shift content left, which broke the original source layout.

This change keeps the original raw line alongside the editor-facing sanitized version and restores the original bytes before upload/save. The editor still sanitizes the content for display in VS Code, but the saved member keeps the exact legacy bytes intact.

This also adds regression coverage for multi-line members and control bytes in different fixed positions.

@Balrocj
Balrocj deployed to testing_environment November 5, 2026 15:31 — with GitHub Actions Active
@bobcozzi

bobcozzi commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Thanks for digging into this, @Balrocj — but I'm not in favor of the direction here, and I want to explain why before this goes further.

On preserving 5250 hex codes in source: I'm against it. They aren't source content — they're 5250 datastream attribute bytes that only ever "worked" in SEU as an accidental by-product of the protocol. When this came up in #2057, the project's direction was explicit:

What are your options?

  1. You can continue to use an older version of Code for IBM i that will have SEU colour support.
  2. You can (and should) look into removing SEU colour bytes from your source code.

And on the preservation question specifically, @worksofliam was direct: "Honestly, no. I don't plan on supporting this at all. We cannot confirm that it will work due to the fact that the conversion between EBCDIC -> UTF8 (which happens on the server) removes those characters." The sanctioned remediation path is to strip them

Notably, jkyeung also reported in that thread that x'25' and x'3F' already don't survive the round trip today (linefeed and \Z respectively) — so "don't alter them" has never actually held; the question has only ever been how they get altered. This PR goes the opposite way from the 2057 direction: it reintroduces preservation machinery (baseLegacySource, position-based byte restoration) into the core save path. Beyond the philosophical concern, the implementation has real problems:

It reverts the NULL fix from PR #3510 without addressing the original issue. #3510 fixed members that fail to open entirely — VS Code classifies the document as binary when the text stream contains U+0000. The client-side .replace(/[\u0000\u0016\u001A]/g, ' ') on open still sanitizes for display, but the NULL case is not actually preserved on save: restoreLegacySourceControlChars restores the \u0000 into the line, then escapeString(..., true) still hits case '\0': return '\0' — Db2 SQL literals don't have backslash escapes, so the member gets back a literal \ + 0 pair, not x'00'. Your tests only cover \u0016/\u001A; add a \u0000 case and it will fail. So even on its own terms, this doesn't preserve the byte class that motivated #3510.

Position-based restore breaks on any edit. Mapping legacySourceLines[i] → sourceData[i] by index means inserting or deleting a line shifts every subsequent line's control bytes onto the wrong neighbor. Combined with trimEnd() and record-length truncation, saved content can silently diverge from what the user saw.

It only covers the legacy RUNSQLSTM path. The newer Mapepire save path (#3462, useMapepireForSourceMemberSaves) never reads baseLegacySource, so preservation now depends on a performance setting — two different save behaviors for the same member.

What I'd support instead: extend the read-side filtering in #3510 (host-side TRANSLATE, or client-side equivalent) so members with these other bytes always open cleanly as text, and let saves just save — exactly the "remove the bytes from your source" option Liam recommended. If a warning when such bytes are detected is desired (also raised in the 2057 discussion) I'd support that, but not a silent byte-preservation machinery in the save path.

For me, the bottom line, filter them out on download/read or change them on the host. Preservation is not the right option IMHO.

@Balrocj

Balrocj commented Nov 7, 2026

Copy link
Copy Markdown
Author

@bobcozzi thanks for the detailed review. My original goal was not to preserve SEU colour attributes, but to prevent the save flow from corrupting source by converting these control bytes into \Z or otherwise shifting fixed-format content.

I agree that restoring the original bytes during save is not the right approach, particularly given line edits and the Mapepire path. Replacing the affected control bytes with spaces on read preserves the fixed-format layout while following the established project direction of treating them as non-source data.

I'll rework the PR to extend the existing read-side sanitization, remove the save-time restoration logic, and add regression coverage for the relevant control bytes, including U+0000.

@Balrocj
Balrocj deployed to testing_environment November 8, 2026 17:52 — with GitHub Actions Active
@Balrocj
Balrocj marked this pull request as draft November 8, 2026 17:54

This branch was successfully deployed

1 active deployment
testing_environment — cb66a3f9 Deployed Nov 8, 2026 by Balrocj via Test runner #777
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