fix sorting

Author: jonaraphael

src/CsvEditorProvider.ts

@@ -160,31 +160,57 @@ class CsvEditorController {
     const data = result.data as string[][];
     const hadRows = data.length;
     const hadColsAtRow = (data[row] ? data[row].length : 0);
+
+    const { data: nextData, trimmed, createdRow, createdCol } = this.mutateDataForEdit(data, row, col, value);
+
+    const newCsvText = Papa.unparse(nextData, { delimiter: separator });
+
+    const fullRange = new vscode.Range(
+      0, 0,
+      this.document.lineCount,
+      this.document.lineCount ? this.document.lineAt(this.document.lineCount - 1).text.length : 0
+    );
+    const edit = new vscode.WorkspaceEdit();
+    edit.replace(this.document.uri, fullRange, newCsvText);
+    await vscode.workspace.applyEdit(edit);
+
+    this.isUpdatingDocument = false;
+    console.log(`CSV: Updated row ${row + 1}, column ${col + 1} to "${value}"`);
+    this.currentWebviewPanel?.webview.postMessage({ type: 'updateCell', row, col, value });
+
+    // Trigger a full re-render if structure may have changed (new row/col created)
+    if (trimmed || createdRow || createdCol || row >= hadRows || col >= hadColsAtRow) {
+      try { this.updateWebviewContent(); } catch (e) { console.error('CSV: refresh failed after structural edit', e); }
+    }
+  }
+
+  // Apply an edit to a 2D data array, enforcing virtual row/cell invariants.
+  // - Empty edits on non-existent virtual row/col are ignored
+  // - Non-empty edits expand rows/cols as needed
+  // - When editing the last row, trailing empty rows are trimmed
+  private mutateDataForEdit(data: string[][], row: number, col: number, value: string): { data: string[][]; trimmed: boolean; createdRow: boolean; createdCol: boolean } {
+    // Work on the same array instance (callers pass freshly parsed data)
+    const hadRows = data.length;
+    const hadColsAtRow = (data[row] ? data[row].length : 0);
     const wasEditingLastRow = row >= (data.length - 1);
 
     const rowExists = row < data.length;
-    const colExists = rowExists && col < data[row].length;
+    const colExists = rowExists && col < (data[row]?.length ?? 0);
 
     if (value === '') {
       if (!rowExists) {
-        // Do not expand file with an empty virtual row
-        this.isUpdatingDocument = false;
-        return;
+        return { data, trimmed: false, createdRow: false, createdCol: false };
       }
       if (!colExists) {
-        // Do not expand row width with an empty virtual cell
-        this.isUpdatingDocument = false;
-        return;
+        return { data, trimmed: false, createdRow: false, createdCol: false };
       }
-      // Existing cell: clear value
       data[row][col] = '';
     } else {
-      // Non-empty value: expand as needed and set
       while (data.length <= row) data.push([]);
       while (data[row].length <= col) data[row].push('');
       data[row][col] = value;
     }
-    // If we edited the (previous) last row, trim trailing empty rows recursively
+
     let trimmed = false;
     if (wasEditingLastRow) {
       const isRowEmpty = (arr: string[] | undefined) => {
@@ -200,25 +226,12 @@ class CsvEditorController {
       }
     }
 
-    const newCsvText = Papa.unparse(data, { delimiter: separator });
-
-    const fullRange = new vscode.Range(
-      0, 0,
-      this.document.lineCount,
-      this.document.lineCount ? this.document.lineAt(this.document.lineCount - 1).text.length : 0
-    );
-    const edit = new vscode.WorkspaceEdit();
-    edit.replace(this.document.uri, fullRange, newCsvText);
-    await vscode.workspace.applyEdit(edit);
-
-    this.isUpdatingDocument = false;
-    console.log(`CSV: Updated row ${row + 1}, column ${col + 1} to "${value}"`);
-    this.currentWebviewPanel?.webview.postMessage({ type: 'updateCell', row, col, value });
-
-    // Trigger a full re-render if structure may have changed (new row/col created)
-    if (trimmed || row >= hadRows || col >= hadColsAtRow) {
-      try { this.updateWebviewContent(); } catch (e) { console.error('CSV: refresh failed after structural edit', e); }
-    }
+    return {
+      data,
+      trimmed,
+      createdRow: value !== '' && row >= hadRows,
+      createdCol: value !== '' && col >= hadColsAtRow
+    };
   }
 
   private async handleSave() {
@@ -347,7 +360,8 @@ class CsvEditorController {
 
     const text   = this.document.getText();
     const result = Papa.parse(text, { dynamicTyping: false, delimiter: separator });
-    const rows   = result.data as string[][];
+    // Exclude virtual/trailing empty rows from sort input
+    const rows   = this.trimTrailingEmptyRows(result.data as string[][]);
     const treatHeader  = this.getEffectiveHeader(rows, this.getHiddenRows());
 
     const offset = Math.min(Math.max(0, hidden), rows.length);
@@ -362,9 +376,26 @@ class CsvEditorController {
     }
 
     const cmp = (a: string, b: string) => {
-      const na = parseFloat(a), nb = parseFloat(b);
+      const sa = (a ?? '').trim();
+      const sb = (b ?? '').trim();
+      const aEmpty = sa === '';
+      const bEmpty = sb === '';
+      if (aEmpty && bEmpty) return 0;
+      if (aEmpty) return 1; // empty sorts last
+      if (bEmpty) return -1;
+
+      // Dates take precedence over numeric compare (avoid parseFloat on ISO)
+      const aIsDate = this.isDate(sa);
+      const bIsDate = this.isDate(sb);
+      if (aIsDate && bIsDate) {
+        const da = Date.parse(sa);
+        const db = Date.parse(sb);
+        if (!isNaN(da) && !isNaN(db)) return da - db;
+      }
+
+      const na = parseFloat(sa), nb = parseFloat(sb);
       if (!isNaN(na) && !isNaN(nb)) return na - nb;
-      return a.localeCompare(b, undefined, { sensitivity: 'base' });
+      return sa.localeCompare(sb, undefined, { sensitivity: 'base' });
     };
 
     body.sort((r1, r2) => {
@@ -374,7 +405,19 @@ class CsvEditorController {
 
     const prefix = rows.slice(0, offset);
     const combined = treatHeader ? [...prefix, header, ...body] : [...prefix, ...body];
-    const newCsv = Papa.unparse(combined, { delimiter: separator });
+
+    // Sanitize before unparse: ensure undefined/null/NaN become empty strings
+    const sanitized: string[][] = combined.map(r => r.map((v: any) => {
+      if (v === undefined || v === null) return '';
+      const t = typeof v;
+      if (t === 'number') {
+        return Number.isNaN(v) ? '' : String(v);
+      }
+      const s = String(v);
+      return s.toLowerCase() === 'nan' ? '' : s;
+    }));
+
+    const newCsv = Papa.unparse(sanitized, { delimiter: separator });
 
     const fullRange = new vscode.Range(
       0, 0,
@@ -1038,10 +1081,75 @@ export class CsvEditorProvider implements vscode.CustomTextEditorProvider {
 
   // Test helpers to access internal utilities without VS Code runtime
   public static __test = {
+    // Pure helper mirroring sort behavior; returns combined rows after sort.
+    sortByColumn(rows: string[][], index: number, ascending: boolean, treatHeader: boolean, hiddenRows: number): string[][] {
+      // Trim trailing empty rows like runtime before sorting
+      const isEmpty = (r: string[] | undefined) => {
+        if (!r || r.length === 0) return true;
+        for (let i = 0; i < r.length; i++) { if ((r[i] ?? '') !== '') return false; }
+        return true;
+      };
+      let end = rows.length;
+      while (end > 0 && isEmpty(rows[end - 1])) { end--; }
+      const trimmed = rows.slice(0, end);
+
+      const offset = Math.min(Math.max(0, hiddenRows), trimmed.length);
+      let header: string[] = [];
+      let body:   string[][] = [];
+      if (treatHeader && offset < trimmed.length) {
+        header = trimmed[offset];
+        body   = trimmed.slice(offset + 1);
+      } else {
+        body   = trimmed.slice(offset);
+      }
+      const isDateStr = (v: string) => {
+        const s = (v ?? '').trim();
+        if (!s) return false;
+        const isoDate = /^\d{4}-\d{2}-\d{2}(?:[ T]\d{2}:\d{2}(?::\d{2})?(?:Z|[+-]\d{2}:?\d{2})?)?$/;
+        const isoSlash = /^\d{4}\/\d{2}\/\d{2}$/;
+        return isoDate.test(s) || isoSlash.test(s);
+      };
+      const cmp = (a: string, b: string) => {
+        const sa = (a ?? '').trim();
+        const sb = (b ?? '').trim();
+        const aEmpty = sa === '';
+        const bEmpty = sb === '';
+        if (aEmpty && bEmpty) return 0;
+        if (aEmpty) return 1; // empty sorts last
+        if (bEmpty) return -1;
+        if (isDateStr(sa) && isDateStr(sb)) {
+          const da = Date.parse(sa);
+          const db = Date.parse(sb);
+          if (!isNaN(da) && !isNaN(db)) return da - db;
+        }
+        const na = parseFloat(sa), nb = parseFloat(sb);
+        if (!isNaN(na) && !isNaN(nb)) return na - nb;
+        return sa.localeCompare(sb, undefined, { sensitivity: 'base' });
+      };
+      body.sort((r1, r2) => {
+        const diff = cmp(r1[index] ?? '', r2[index] ?? '');
+        return ascending ? diff : -diff;
+      });
+      const prefix = trimmed.slice(0, offset);
+
+      // Apply same sanitation used before unparse in runtime path
+      const combined = (treatHeader ? [...prefix, header, ...body] : [...prefix, ...body]).map(r => r.map((v: any) => {
+        if (v === undefined || v === null) return '';
+        const t = typeof v;
+        if (t === 'number') return Number.isNaN(v) ? '' : String(v);
+        const s = String(v);
+        return s.toLowerCase() === 'nan' ? '' : s;
+      }));
+      return combined;
+    },
     computeColumnWidths(data: string[][]): number[] {
       const c: any = new (CsvEditorController as any)({} as any);
       return c.computeColumnWidths(data);
     },
+    mutateDataForEdit(data: string[][], row: number, col: number, value: string): { data: string[][]; trimmed: boolean; createdRow: boolean; createdCol: boolean } {
+      const c: any = new (CsvEditorController as any)({} as any);
+      return c.mutateDataForEdit(data, row, col, value);
+    },
     isDate(v: string): boolean {
       const c: any = new (CsvEditorController as any)({} as any);
       return c.isDate(v);
@@ -1104,6 +1212,13 @@ export class CsvEditorProvider implements vscode.CustomTextEditorProvider {
       } catch {
         return { chunkCount: 0, hasTable: false };
       }
+    },
+    generateTableAndChunksRaw(data: string[][], treatHeader: boolean, addSerialIndex: boolean, hiddenRows: number): { tableHtml: string; chunks: string[] } {
+      const c: any = new (CsvEditorController as any)({} as any);
+      const result = c.generateTableAndChunks(data, treatHeader, addSerialIndex, hiddenRows);
+      let chunks: string[] = [];
+      try { chunks = JSON.parse(result.chunksJson); } catch {}
+      return { tableHtml: result.tableHtml, chunks };
     }
   };
 }

src/test/edit-mutate.test.ts

@@ -0,0 +1,51 @@
+import assert from 'assert';
+import { describe, it } from 'node:test';
+import Module from 'module';
+
+// Stub 'vscode' before importing provider
+const originalRequire = Module.prototype.require;
+Module.prototype.require = function (id: string) {
+  if (id === 'vscode') {
+    return {
+      window: { activeColorTheme: { kind: 1 } },
+      ColorThemeKind: { Dark: 1 }
+    } as any;
+  }
+  return originalRequire.apply(this, arguments as any);
+};
+
+import { CsvEditorProvider } from '../CsvEditorProvider';
+
+describe('Edit mutate invariants', () => {
+  it('does not promote virtual row on empty edit', () => {
+    const res = CsvEditorProvider.__test.mutateDataForEdit([], 0, 0, '');
+    assert.deepStrictEqual(res.data, []);
+    assert.strictEqual(res.createdRow, false);
+  });
+
+  it('does not promote virtual cell on empty edit into new column', () => {
+    const init = [['a']];
+    const res = CsvEditorProvider.__test.mutateDataForEdit(init.map(r => [...r]), 0, 2, '');
+    assert.deepStrictEqual(res.data, [['a']]);
+    assert.strictEqual(res.createdCol, false);
+  });
+
+  it('non-empty edit expands rows and columns as needed', () => {
+    const a = CsvEditorProvider.__test.mutateDataForEdit([], 0, 0, 'x');
+    assert.deepStrictEqual(a.data, [['x']]);
+    assert.strictEqual(a.createdRow, true);
+    assert.strictEqual(a.createdCol, true);
+
+    const b = CsvEditorProvider.__test.mutateDataForEdit([['a']], 0, 2, 'v');
+    assert.deepStrictEqual(b.data, [['a', '', 'v']]);
+    assert.strictEqual(b.createdCol, true);
+  });
+
+  it('trims trailing empty rows when editing last row', () => {
+    const init = [['a'], ['']];
+    const res = CsvEditorProvider.__test.mutateDataForEdit(init.map(r => [...r]), 1, 0, '');
+    assert.deepStrictEqual(res.data, [['a']]);
+    assert.strictEqual(res.trimmed, true);
+  });
+});
+

src/test/sort-dates.test.ts

@@ -0,0 +1,63 @@
+import assert from 'assert';
+import { describe, it } from 'node:test';
+import Module from 'module';
+
+// Stub 'vscode' prior to loading provider (theme checks)
+const originalRequire = Module.prototype.require;
+Module.prototype.require = function (id: string) {
+  if (id === 'vscode') {
+    return {
+      window: { activeColorTheme: { kind: 1 } },
+      ColorThemeKind: { Dark: 1 }
+    } as any;
+  }
+  return originalRequire.apply(this, arguments as any);
+};
+
+import { CsvEditorProvider } from '../CsvEditorProvider';
+
+describe('Date column sort', () => {
+  it('sorts ISO yyyy-mm-dd ascending and descending', () => {
+    const input = [
+      ['2017-02-18'],
+      ['2017-12-04'],
+      ['2017-02-10'],
+      ['2017-04-16'],
+      ['2017-06-22'],
+      ['2017-04-08'],
+      ['2017-06-14'],
+      ['2017-08-20']
+    ];
+
+    const asc = CsvEditorProvider.__test.sortByColumn(input, 0, true, false, 0).map(r => r[0]);
+    assert.deepStrictEqual(asc, [
+      '2017-02-10',
+      '2017-02-18',
+      '2017-04-08',
+      '2017-04-16',
+      '2017-06-14',
+      '2017-06-22',
+      '2017-08-20',
+      '2017-12-04'
+    ]);
+
+    const desc = CsvEditorProvider.__test.sortByColumn(input, 0, false, false, 0).map(r => r[0]);
+    assert.deepStrictEqual(desc, [
+      '2017-12-04',
+      '2017-08-20',
+      '2017-06-22',
+      '2017-06-14',
+      '2017-04-16',
+      '2017-04-08',
+      '2017-02-18',
+      '2017-02-10'
+    ]);
+  });
+
+  it('treats empty dates as last in ascending', () => {
+    const input = [ ['2017-01-01'], [''], ['2017-01-03'] ];
+    const asc = CsvEditorProvider.__test.sortByColumn(input, 0, true, false, 0).map(r => r[0]);
+    assert.deepStrictEqual(asc, ['2017-01-01', '2017-01-03', '']);
+  });
+});
+