From 1c90137e7915546d527b1a2aeff44f03a107abcf Mon Sep 17 00:00:00 2001 From: "Andrei.Ovcharenko" Date: Tue, 4 Aug 2026 12:27:23 +0300 Subject: [PATCH 1/2] Stop unwrapping sparse rows that wrapping could not have produced Fit Width and the manual narrow/widen actions unwrap continuation rows before re-laying out a table. The check that decided what counts as a continuation row only looked at the shape of the row: any row that left at least one column empty under a filled one was merged into the row above. Ordinary sparse data has that shape too, so distinct records were silently collapsed into a single row - including automatically, when power auto fit runs after an unrelated edit. Wrapping leaves a checkable trace, so test for it instead of guessing: - It fills a cell's segments from the top, so a segment can never sit under an empty cell. - It is greedy, so it never leaves room for the next token. A row whose first token would still have fitted after the previous segment was never wrapped. The reference width for that test is measured only over rows that fill every column. A continuation row must leave a column empty, so those rows carry the real column width; measuring the candidates too would let a hand-split row widen the very column it is tested against, which is what kept hand-split words such as "scrip" / "t already" joinable. Rows that wrapping did produce are still rejoined, so widening a fitted table still rebuilds the original sentence and repeated fits stay idempotent. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KsNFrStLUQaEZNtCybK4XD --- CHANGELOG.md | 6 ++ src/core.ts | 92 ++++++++++++++++++- test-fixtures/markdown-table-core-golden.json | 62 +++++++++++++ test/core-parity.test.ts | 37 ++++++++ 4 files changed, 192 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87ec474..df72e92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ All notable changes follow [Keep a Changelog](https://keepachangelog.com/en/1.1. ### Fixed +- Fit Width and the manual narrow/widen actions no longer merge sparse rows that wrapping + could not have produced, so distinct records survive power auto fit. Rows that wrapping did + produce are still rejoined. + +### Fixed + - The table core is now a faithful port of the shared JetBrains/Notepad++ engine and matches it byte for byte across the differential corpus. This corrects several behaviours that diverged: - Prose that merely contains a pipe is no longer swallowed into the table and rewritten. diff --git a/src/core.ts b/src/core.ts index 305da98..5a5194d 100644 --- a/src/core.ts +++ b/src/core.ts @@ -1124,7 +1124,77 @@ function nonEmptyCellCount(row: Row): number { return count; } -function isLikelyContinuationRow(row: Row, baseRow: Row, columns: number): boolean { +/** + * The first token of a cell exactly as {@link wrapCellSegments} would split it, so Markdown links + * and code spans stay whole. + */ +function firstWrapToken(cell: string): string { + const value = trim(cell); + if (value.length === 0) return ''; + + let end = 0; + if (value[0] === '`') end = markdownCodeSpanEnd(value, 0); + else if (startsMarkdownLinkAt(value, 0)) end = markdownLinkEnd(value, 0); + else { + while (end < value.length && !isSpace(at(value, end))) end += charCount(codePointAt(value, end)); + } + return value.slice(0, Math.min(end, value.length)); +} + +/** + * Column widths a wrap would have used, measured only over rows that cannot themselves be + * continuations. + * + * A continuation row must leave at least one column empty, so rows that fill every column carry the + * real width. Measuring the continuation candidates too would let a hand-split row widen the very + * column it is tested against. + */ +function wrappingReferenceWidths(table: Table): number[] { + const widths = uniformWidths(table, 1); + for (let rowIndex = 0; rowIndex < table.rows.length; rowIndex += 1) { + const row = table.rows[rowIndex] as Row; + if (!row.separator && rowIndex > table.separatorRow && nonEmptyCellCount(row) !== table.columns) continue; + growWidthsToFit(widths, row, table.columns); + } + return widths; +} + +/** + * Whether `row` could have been produced by wrapping the cells of `previousSegment` at `widths`. + * + * Wrapping leaves a checkable trace. It fills a cell's segments from the top, so a segment never + * sits under an empty one, and it is greedy, so it never leaves room for the next token. A row that + * breaks either rule is ordinary sparse data that merely looks like wrapping output, and merging it + * would destroy a record. + */ +function couldFollowWrappedSegment( + previousSegment: Row | undefined, + row: Row, + columns: number, + widths: readonly number[], +): boolean { + if (previousSegment === undefined || previousSegment.cells.length < columns) return false; + + for (let column = 0; column < columns; column += 1) { + const cell = row.cells[column] as string; + if (!cellHasText(cell)) continue; + + const previousCell = previousSegment.cells[column] as string; + if (!cellHasText(previousCell)) return false; + + const width = column < widths.length ? (widths[column] as number) : 0; + if (displayWidth(previousCell) + 1 + displayWidth(firstWrapToken(cell)) <= width) return false; + } + return true; +} + +function isLikelyContinuationRow( + row: Row, + baseRow: Row, + previousSegment: Row | undefined, + columns: number, + widths: readonly number[], +): boolean { if (columns < 2 || row.cells.length < columns || baseRow.cells.length < columns) return false; const nonEmpty = nonEmptyCellCount(row); @@ -1138,7 +1208,9 @@ function isLikelyContinuationRow(row: Row, baseRow: Row, columns: number): boole } const requiredAnchors = Math.max(1, Math.floor(columns / 3)); - return emptyWhereBaseHasText >= requiredAnchors; + if (emptyWhereBaseHasText < requiredAnchors) return false; + + return couldFollowWrappedSegment(previousSegment, row, columns, widths); } function copyRow(row: Row): Row { @@ -1213,15 +1285,18 @@ function continuationRowsToPreserve(table: Table, originalTargetRow: number): bo if (table.separatorRow === -1 || originalTargetRow < 0 || originalTargetRow >= table.rows.length) return preserve; const continuationBaseForRow = Array.from({ length: table.rows.length }, () => -1); + const widths = wrappingReferenceWidths(table); let baseRowIndex = -1; let baseRow: Row | undefined; + let previousSegment: Row | undefined; for (let rowIndex = 0; rowIndex < table.rows.length; rowIndex += 1) { const row = table.rows[rowIndex] as Row; if (row.separator || rowIndex <= table.separatorRow || baseRowIndex === -1 || baseRow === undefined - || !isLikelyContinuationRow(row, baseRow, table.columns)) { + || !isLikelyContinuationRow(row, baseRow, previousSegment, table.columns, widths)) { if (!row.separator && rowIndex > table.separatorRow) { baseRowIndex = rowIndex; baseRow = copyRow(row); + previousSegment = copyRow(row); } continue; } @@ -1230,6 +1305,7 @@ function continuationRowsToPreserve(table: Table, originalTargetRow: number): bo for (let column = 0; column < table.columns; column += 1) { baseRow.cells[column] = appendContinuationCell(baseRow.cells[column] as string, row.cells[column] as string); } + previousSegment = copyRow(row); } const targetBaseRow = continuationBaseForRow[originalTargetRow] as number; @@ -1244,15 +1320,20 @@ function unwrapContinuationRows(table: Table, originalTargetRow: number): number if (table.separatorRow === -1) return originalTargetRow; const preserveContinuationRows = continuationRowsToPreserve(table, originalTargetRow); + const widths = wrappingReferenceWidths(table); const unwrappedRows: Row[] = []; let targetRow = originalTargetRow; let baseRowIndex = -1; + let previousSegment: Row | undefined; for (let rowIndex = 0; rowIndex < table.rows.length; rowIndex += 1) { const row = table.rows[rowIndex] as Row; if (row.separator || rowIndex <= table.separatorRow || preserveContinuationRows[rowIndex] || baseRowIndex === -1 - || !isLikelyContinuationRow(row, unwrappedRows[baseRowIndex] as Row, table.columns)) { + || !isLikelyContinuationRow(row, unwrappedRows[baseRowIndex] as Row, previousSegment, table.columns, widths)) { if (rowIndex === originalTargetRow) targetRow = unwrappedRows.length; - if (!row.separator && rowIndex > table.separatorRow) baseRowIndex = unwrappedRows.length; + if (!row.separator && rowIndex > table.separatorRow) { + baseRowIndex = unwrappedRows.length; + previousSegment = copyRow(row); + } unwrappedRows.push(row); continue; } @@ -1263,6 +1344,7 @@ function unwrapContinuationRows(table: Table, originalTargetRow: number): number for (let column = 0; column < table.columns; column += 1) { baseRow.cells[column] = appendContinuationCell(baseRow.cells[column] as string, row.cells[column] as string); } + previousSegment = copyRow(row); } table.rows = unwrappedRows; diff --git a/test-fixtures/markdown-table-core-golden.json b/test-fixtures/markdown-table-core-golden.json index bd52186..d624b5d 100644 --- a/test-fixtures/markdown-table-core-golden.json +++ b/test-fixtures/markdown-table-core-golden.json @@ -628,6 +628,68 @@ ], "targetRow": 2, "targetColumn": 0 + }, + { + "name": "manual resize keeps sparse rows that wrapping could not have produced", + "action": "WIDEN_COLUMN", + "row": 4, + "column": 0, + "input": [ + "| Name | Note |", + "| --- | --- |", + "| Alice | short |", + "| | second |", + "| Bob | a much longer value here |" + ], + "lines": [ + "| Name | Note |", + "| ------ | ------------------------ |", + "| Alice | short |", + "| | second |", + "| Bob | a much longer value here |" + ], + "targetRow": 4, + "targetColumn": 0 + }, + { + "name": "manual resize still rejoins rows that wrapping produced", + "action": "WIDEN_COLUMN", + "row": 2, + "column": 0, + "input": [ + "| Key | Description |", + "| --- | ------------------ |", + "| x | alpha beta gamma |", + "| | delta epsilon zeta |" + ], + "lines": [ + "| Key | Description |", + "| ---- | ------------------ |", + "| x | alpha beta gamma |", + "| | delta epsilon zeta |" + ], + "targetRow": 2, + "targetColumn": 0 + }, + { + "name": "a segment under an empty cell is never treated as wrapping output", + "action": "WIDEN_COLUMN", + "row": 3, + "column": 0, + "input": [ + "| A | B |", + "| --- | --- |", + "| a | |", + "| | b |" + ], + "lines": [ + "| A | B |", + "| ---- | --- |", + "| a | |", + "| | b |" + ], + "targetRow": 3, + "targetColumn": 0 } ] } diff --git a/test/core-parity.test.ts b/test/core-parity.test.ts index a176352..55b80cc 100644 --- a/test/core-parity.test.ts +++ b/test/core-parity.test.ts @@ -108,3 +108,40 @@ test('delimited conversion keeps cell text verbatim and reports the shared messa assert.equal(fromDelimited('').message, 'No CSV or TSV data found'); assert.equal(fromDelimited('no delimiter').message, 'No CSV or TSV data found'); }); + +test('fitting keeps sparse rows that wrapping could not have produced', () => { + // The third row sets the column width, so "second" would still have fitted after "short". + // Wrapping is greedy and never leaves that room, so these are two records, not one wrapped row. + const table = [ + '| Name | Note |', + '| ----- | ------------------------ |', + '| Alice | short |', + '| | second |', + '| Bob | a much longer value here |', + ]; + const result = applyWrappedToWidth(table, 0, 0, 200); + assert.deepEqual(result.lines, table); + assert.equal(result.changed, false); + + // Wrapping fills a cell's segments from the top, so "b" cannot be the second segment of an + // empty cell. + const underEmpty = applyWrappedToWidth(['| A | B |', '| --- | --- |', '| a | |', '| | b |'], 0, 0, 200); + assert.deepEqual(underEmpty.lines, ['| A | B |', '| --- | --- |', '| a | |', '| | b |']); +}); + +test('fitting still rejoins rows that wrapping produced', () => { + const wrapped = [ + '| Key | Description |', + '| --- | ------------------ |', + '| x | alpha beta gamma |', + '| | delta epsilon zeta |', + ]; + const result = applyWrappedToWidth(wrapped, 0, 0, 120); + assert.equal(result.lines.length, 3); + assert.ok((result.lines[2] ?? '').includes('alpha beta gamma delta epsilon zeta'), result.lines.join('\n')); +}); + +test('a hand split word is still rejoined even when the table is not aligned', () => { + const result = applyWrappedToWidth(['| A | B |', '| --- | --- |', '| scrip | keep |', '| t already | |'], 0, 0, 80); + assert.ok((result.lines[2] ?? '').includes('script already'), result.lines.join('\n')); +}); From 2f918881e3ec38bc5d23e12dd445dbbaf40f826f Mon Sep 17 00:00:00 2001 From: "Andrei.Ovcharenko" Date: Tue, 4 Aug 2026 12:45:44 +0300 Subject: [PATCH 2/2] Measure the wrap reference width from body rows and whole cells Two ways the continuation check could reject a genuine continuation: - Header rows are never wrapped, so when a fit pushes body cells below the header width, formatTable widens the rendered column back to the header. The reference width then described a width the body was never split at, and a later fit refused to rejoin the record. Measure only body rows now. - Wrapping hard-splits an over-wide link or code span mid-token, so a fragment such as "[x y](ur" no longer parses as a link. Re-tokenising it under-measured the segment. Compare the whole cell instead, which needs no tokenising and is the conservative direction: it only refuses a merge when the entire cell would still have fitted after the previous segment. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KsNFrStLUQaEZNtCybK4XD --- src/core.ts | 43 ++++++++++++++-------------------------- test/core-parity.test.ts | 23 +++++++++++++++++++++ 2 files changed, 38 insertions(+), 28 deletions(-) diff --git a/src/core.ts b/src/core.ts index 5a5194d..7af7374 100644 --- a/src/core.ts +++ b/src/core.ts @@ -1125,35 +1125,17 @@ function nonEmptyCellCount(row: Row): number { } /** - * The first token of a cell exactly as {@link wrapCellSegments} would split it, so Markdown links - * and code spans stay whole. - */ -function firstWrapToken(cell: string): string { - const value = trim(cell); - if (value.length === 0) return ''; - - let end = 0; - if (value[0] === '`') end = markdownCodeSpanEnd(value, 0); - else if (startsMarkdownLinkAt(value, 0)) end = markdownLinkEnd(value, 0); - else { - while (end < value.length && !isSpace(at(value, end))) end += charCount(codePointAt(value, end)); - } - return value.slice(0, Math.min(end, value.length)); -} - -/** - * Column widths a wrap would have used, measured only over rows that cannot themselves be - * continuations. + * Column widths a wrap would have used, measured only over body rows that fill every column. * - * A continuation row must leave at least one column empty, so rows that fill every column carry the - * real width. Measuring the continuation candidates too would let a hand-split row widen the very - * column it is tested against. + * Header rows are never wrapped, so a header wider than the wrap target would report a width the + * body was never split at. A continuation row must leave at least one column empty, so measuring + * the candidates too would let a hand-split row widen the very column it is tested against. */ function wrappingReferenceWidths(table: Table): number[] { const widths = uniformWidths(table, 1); - for (let rowIndex = 0; rowIndex < table.rows.length; rowIndex += 1) { + for (let rowIndex = table.separatorRow + 1; rowIndex < table.rows.length; rowIndex += 1) { const row = table.rows[rowIndex] as Row; - if (!row.separator && rowIndex > table.separatorRow && nonEmptyCellCount(row) !== table.columns) continue; + if (row.separator || nonEmptyCellCount(row) !== table.columns) continue; growWidthsToFit(widths, row, table.columns); } return widths; @@ -1163,9 +1145,14 @@ function wrappingReferenceWidths(table: Table): number[] { * Whether `row` could have been produced by wrapping the cells of `previousSegment` at `widths`. * * Wrapping leaves a checkable trace. It fills a cell's segments from the top, so a segment never - * sits under an empty one, and it is greedy, so it never leaves room for the next token. A row that - * breaks either rule is ordinary sparse data that merely looks like wrapping output, and merging it - * would destroy a record. + * sits under an empty one, and it never splits a cell that fits, so a cell that would still have + * fitted after the previous segment was never wrapped away from it. A row that breaks either rule + * is ordinary sparse data that merely looks like wrapping output, and merging it would destroy a + * record. + * + * The second test deliberately measures the whole cell rather than its first token. A segment can + * be a fragment of a construct that was hard-split mid-token, and re-tokenising such a fragment + * would under-measure it and reject a genuine continuation. */ function couldFollowWrappedSegment( previousSegment: Row | undefined, @@ -1183,7 +1170,7 @@ function couldFollowWrappedSegment( if (!cellHasText(previousCell)) return false; const width = column < widths.length ? (widths[column] as number) : 0; - if (displayWidth(previousCell) + 1 + displayWidth(firstWrapToken(cell)) <= width) return false; + if (displayWidth(previousCell) + 1 + displayWidth(trim(cell)) <= width) return false; } return true; } diff --git a/test/core-parity.test.ts b/test/core-parity.test.ts index 55b80cc..7d961ac 100644 --- a/test/core-parity.test.ts +++ b/test/core-parity.test.ts @@ -145,3 +145,26 @@ test('a hand split word is still rejoined even when the table is not aligned', ( const result = applyWrappedToWidth(['| A | B |', '| --- | --- |', '| scrip | keep |', '| t already | |'], 0, 0, 80); assert.ok((result.lines[2] ?? '').includes('script already'), result.lines.join('\n')); }); + +test('fitting rejoins a body that was wrapped below the header width', () => { + // The header is wider than the wrap target, so the rendered column is wider than the width the + // body segments were actually split at. + const narrow = applyWrappedToWidth( + ['| Identifier | Description |', '| --- | --- |', '| x | alpha beta gamma delta epsilon |'], + 2, 0, 15, + ); + assert.ok(narrow.lines.length > 3, narrow.lines.join('\n')); + assert.equal(applyWrappedToWidth(narrow.lines, 0, 0, 200).lines.length, 3); +}); + +test('fitting rejoins constructs that were hard split mid token', () => { + // Wrapping cuts an over-wide link mid-token, so a fragment no longer parses as a link. + const narrow = applyWrappedToWidth( + ['| A | B |', '| --- | --- |', '| [x y](url) [x y](url) | a |'], + 2, 0, 18, + ); + assert.ok(narrow.lines.length > 3, narrow.lines.join('\n')); + const wide = applyWrappedToWidth(narrow.lines, 0, 0, 200); + assert.equal(wide.lines.length, 3); + assert.ok((wide.lines[2] ?? '').includes('[x y](url) [x y](url)'), wide.lines.join('\n')); +});