Repository navigation
Conversation
|
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?
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. |
|
@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. |
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.