Stop unwrapping sparse rows that wrapping could not have produced

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.

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;

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
     }
   ]
 }

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'));
+});