From f1b25501b1f5b707a461cfb00e7c326156387e58 Mon Sep 17 00:00:00 2001 From: ryanmelt-agent Date: Thu, 14 May 2026 19:25:40 +0000 Subject: [PATCH 1/2] fix(widgets): show embedded newlines in string telemetry values Newlines, carriage returns, and tabs embedded in string telemetry values were silently truncating the displayed cell at the first control char. Escape them as \n / \r / \t for single-line displays such as Packet Viewer, and let TEXTBOX widgets keep raw newlines so v-textarea renders multi-line output. The escape uses a single module-level RegExp.test fast path so values without embedded whitespace pay only one cheap check (~30ns/call) before returning the original string. JSON.stringify output is now preserved verbatim rather than having literal "\n" sequences silently stripped. Fixes #154 Co-Authored-By: Paperclip --- .../src/widgets/FormatValueBase.js | 38 ++++++++++++-- .../src/widgets/TextboxWidget.vue | 3 ++ .../openc3-vue-common/src/widgets/VWidget.js | 7 ++- playwright/tests/packet-viewer.p.spec.ts | 52 +++++++++++++++++++ 4 files changed, 95 insertions(+), 5 deletions(-) diff --git a/openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/FormatValueBase.js b/openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/FormatValueBase.js index 35bc4c2664..3e2c6ec9aa 100644 --- a/openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/FormatValueBase.js +++ b/openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/FormatValueBase.js @@ -16,29 +16,59 @@ */ import 'sprintf-js' +// Match any whitespace control character we want to make visible in +// single-line displays. Kept as a module-level constant so the regex is +// compiled once and reused across every formatValueBase call. +const WHITESPACE_CONTROL_REGEX = /[\n\r\t]/ +const WHITESPACE_CONTROL_REGEX_G = /[\n\r\t]/g export default { methods: { - formatValueBase(value, formatString) { + formatValueBase(value, formatString, options) { if (this.isJsonString(value)) { return this.formatJsonString(value) } + const preserveWhitespace = options && options.preserveWhitespace === true if (Array.isArray(value)) { - return JSON.stringify(value).replace(/\\n/g, '') + return this.escapeWhitespace(JSON.stringify(value), preserveWhitespace) } if (this.isObject(value)) { - return JSON.stringify(value).replace(/\\n/g, '') + return this.escapeWhitespace(JSON.stringify(value), preserveWhitespace) } if (formatString && value) { if (typeof value === 'bigint') { return this.formatBigInt(value, formatString) } - return sprintf(formatString, value) + return this.escapeWhitespace( + sprintf(formatString, value), + preserveWhitespace, + ) } if (value === null || value === undefined) { return 'null' } + if (typeof value === 'string') { + return this.escapeWhitespace(value, preserveWhitespace) + } return String(value) }, + // Make embedded newlines, carriage returns, and tabs visible in + // single-line displays (e.g. Packet Viewer) by replacing them with their + // escape sequences. Pass preserveWhitespace=true to return the value + // unchanged for multi-line displays such as the TEXTBOX widget. + escapeWhitespace(value, preserveWhitespace) { + if (preserveWhitespace || typeof value !== 'string') { + return value + } + // Cheap test first so we only allocate a new string when needed. + if (!WHITESPACE_CONTROL_REGEX.test(value)) { + return value + } + return value.replace(WHITESPACE_CONTROL_REGEX_G, (m) => { + if (m === '\n') return '\\n' + if (m === '\r') return '\\r' + return '\\t' + }) + }, // sprintf-js doesn't support BigInt values so we handle common // integer format specifiers manually to preserve full precision formatBigInt(value, formatString) { diff --git a/openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/TextboxWidget.vue b/openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/TextboxWidget.vue index a9d02a3153..895246aafe 100644 --- a/openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/TextboxWidget.vue +++ b/openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/TextboxWidget.vue @@ -66,6 +66,9 @@ export default { return { width: 200, height: 200, + // Let v-textarea render embedded newlines as actual line breaks + // rather than the escaped "\n" sequence used by single-line displays. + preserveWhitespace: true, } }, computed: { diff --git a/openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/VWidget.js b/openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/VWidget.js index 02b6248ae1..ac1c65f1c6 100644 --- a/openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/VWidget.js +++ b/openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/VWidget.js @@ -43,6 +43,9 @@ export default { emits: ['addItem', 'deleteItem', 'open'], data() { return { + // Subclasses (e.g. TextboxWidget) set this true to let embedded + // newlines / tabs pass through to multi-line displays unchanged. + preserveWhitespace: false, appliedTimeZone: 'local', curValue: null, prevValue: null, @@ -214,7 +217,9 @@ export default { this.appliedTimeZone, ) } - return this.formatValueBase(value, this.formatString) + return this.formatValueBase(value, this.formatString, { + preserveWhitespace: this.preserveWhitespace, + }) } catch (e) { // eslint-disable-next-line no-console console.log( diff --git a/playwright/tests/packet-viewer.p.spec.ts b/playwright/tests/packet-viewer.p.spec.ts index 9019502a50..bf50dbcfbc 100644 --- a/playwright/tests/packet-viewer.p.spec.ts +++ b/playwright/tests/packet-viewer.p.spec.ts @@ -150,6 +150,58 @@ test('gets details with right click', async ({ page, utils }) => { ) }) +// Regression for openc3#154: newlines embedded in a string telemetry value +// must be visible in the single-line Packet Viewer cell (escaped as \n) and +// not silently truncate everything after the first newline. +test('displays embedded newlines in string telemetry values', async ({ + page, + utils, +}) => { + // 1) Override INST HEALTH_STATUS ASCIICMD with a multi-line string. We do + // this via Script Runner because the cmd-tlm API does not expose + // override_tlm directly over HTTP. + await page.goto('/tools/scriptrunner') + await expect(page.locator('.v-app-bar')).toContainText('Script Runner') + await page.locator('[data-test=script-runner-file]').click() + await page.locator('text=New File').click() + await expect(page.locator('textarea')).toHaveText('') + await page + .locator('textarea') + .fill( + `override_tlm("INST HEALTH_STATUS ASCIICMD = \\"line1\\nline2\\nline3\\"")`, + ) + await page.locator('[data-test=start-button]').click() + await expect(page.locator('[data-test=state] input')).toHaveValue( + 'completed', + { timeout: 30000 }, + ) + + // 2) Verify the Packet Viewer renders the embedded newlines as a literal + // `\n` sequence rather than chopping at the first newline. + await page.goto('/tools/packetviewer/INST/HEALTH_STATUS/') + await expect(page.locator('.v-app-bar')).toContainText('Packet Viewer') + await expect + .poll( + async () => + await page.inputValue('tr:has(td div:text-is("ASCIICMD")) input'), + { timeout: 10000 }, + ) + .toBe('line1\\nline2\\nline3') + + // 3) Clear the override so we don't leak state into other tests. + await page.goto('/tools/scriptrunner') + await page.locator('[data-test=script-runner-file]').click() + await page.locator('text=New File').click() + await page + .locator('textarea') + .fill(`normalize_tlm("INST HEALTH_STATUS ASCIICMD")`) + await page.locator('[data-test=start-button]').click() + await expect(page.locator('[data-test=state] input')).toHaveValue( + 'completed', + { timeout: 30000 }, + ) +}) + test('stops posting to the api after closing', async ({ page, utils }) => { await page.goto('/tools/packetviewer/INST/ADCS/') let requestCount = 0 From 96571f9c0c0fd7bc8089ca20126af98afa8cd0c4 Mon Sep 17 00:00:00 2001 From: ryanmelt-agent Date: Thu, 14 May 2026 21:36:05 +0000 Subject: [PATCH 2/2] fix(widgets): satisfy SonarQube findings on PR 3376 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use optional chaining `options?.preserveWhitespace` instead of the short-circuit `options && options.x` pattern (S6582). - Use `String.raw` templates for the literal `\n` / `\r` / `\t` return values in `escapeWhitespace`, and for the embedded Ruby source in the Playwright override test, so backslash escapes appear in the source exactly as they appear in the produced string (S7780). No behavior change; output strings are byte-for-byte identical to the prior implementation. Verified with `pnpm lint` (vue-common) and `pnpm prettier --check` (playwright) — both clean. Co-Authored-By: Paperclip --- .../openc3-vue-common/src/widgets/FormatValueBase.js | 8 ++++---- playwright/tests/packet-viewer.p.spec.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/FormatValueBase.js b/openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/FormatValueBase.js index 3e2c6ec9aa..139d773f60 100644 --- a/openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/FormatValueBase.js +++ b/openc3-cosmos-init/plugins/packages/openc3-vue-common/src/widgets/FormatValueBase.js @@ -27,7 +27,7 @@ export default { if (this.isJsonString(value)) { return this.formatJsonString(value) } - const preserveWhitespace = options && options.preserveWhitespace === true + const preserveWhitespace = options?.preserveWhitespace === true if (Array.isArray(value)) { return this.escapeWhitespace(JSON.stringify(value), preserveWhitespace) } @@ -64,9 +64,9 @@ export default { return value } return value.replace(WHITESPACE_CONTROL_REGEX_G, (m) => { - if (m === '\n') return '\\n' - if (m === '\r') return '\\r' - return '\\t' + if (m === '\n') return String.raw`\n` + if (m === '\r') return String.raw`\r` + return String.raw`\t` }) }, // sprintf-js doesn't support BigInt values so we handle common diff --git a/playwright/tests/packet-viewer.p.spec.ts b/playwright/tests/packet-viewer.p.spec.ts index bf50dbcfbc..eca508519c 100644 --- a/playwright/tests/packet-viewer.p.spec.ts +++ b/playwright/tests/packet-viewer.p.spec.ts @@ -168,7 +168,7 @@ test('displays embedded newlines in string telemetry values', async ({ await page .locator('textarea') .fill( - `override_tlm("INST HEALTH_STATUS ASCIICMD = \\"line1\\nline2\\nline3\\"")`, + String.raw`override_tlm("INST HEALTH_STATUS ASCIICMD = \"line1\nline2\nline3\"")`, ) await page.locator('[data-test=start-button]').click() await expect(page.locator('[data-test=state] input')).toHaveValue( @@ -186,7 +186,7 @@ test('displays embedded newlines in string telemetry values', async ({ await page.inputValue('tr:has(td div:text-is("ASCIICMD")) input'), { timeout: 10000 }, ) - .toBe('line1\\nline2\\nline3') + .toBe(String.raw`line1\nline2\nline3`) // 3) Clear the override so we don't leak state into other tests. await page.goto('/tools/scriptrunner')