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..7af7374 100644 --- a/src/core.ts +++ b/src/core.ts @@ -1124,7 +1124,64 @@ function nonEmptyCellCount(row: Row): number { return count; } -function isLikelyContinuationRow(row: Row, baseRow: Row, columns: number): boolean { +/** + * Column widths a wrap would have used, measured only over body rows that fill every column. + * + * 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 = table.separatorRow + 1; rowIndex < table.rows.length; rowIndex += 1) { + const row = table.rows[rowIndex] as Row; + if (row.separator || 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 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, + 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(trim(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 +1195,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 +1272,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 +1292,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 +1307,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 +1331,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..7d961ac 100644 --- a/test/core-parity.test.ts +++ b/test/core-parity.test.ts @@ -108,3 +108,63 @@ 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')); +}); + +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')); +});