gajus / table

Formats data into a string table.
Other
894 stars 77 forks source link

SyntaxError: Invalid regular expression: /(^.{1,0}(\s+|$))|(^.{1,-1}(\\|/|_|\.|,|;|-))/: numbers out of order in {} quantifier #182

Closed sebinsua closed 2 years ago

sebinsua commented 2 years ago

The following error can be produced by setting wrapWord on a column that only contains empty cells.

1) drawTable
       should not break when wrapWord is set on a column with empty cells:
     SyntaxError: Invalid regular expression: /(^.{1,0}(\s+|$))|(^.{1,-1}(\\|/|_|\.|,|;|-))/: numbers out of order in {} quantifier
      at new RegExp (<anonymous>)
      at calculateStringLengths (src/wrapWord.ts:10:14)
      at Object.wrapWord (src/wrapWord.ts:41:3)
      at Object.wrapCell (src/wrapCell.ts:27:20)
      at Object.calculateCellHeight (src/calculateCellHeight.ts:9:10)
      at /Users/sebinsua/dev/table/src/calculateRowHeights.ts:17:26
      at Array.forEach (<anonymous>)
      at /Users/sebinsua/dev/table/src/calculateRowHeights.ts:16:9
      at Array.map (<anonymous>)
      at Object.calculateRowHeights (src/calculateRowHeights.ts:13:15)
      at Object.table (src/table.ts:44:22)
      at Context.<anonymous> (test/table.ts:29:7)
      at processImmediate (node:internal/timers:464:21)

A test to show this:

describe("drawTable", () => {
  it("should not break when wrapWord is set on a column with empty cells", () => {
    const config: TableUserConfig = {
      columnDefault: {
        wrapWord: true,
      },
    };

    expect(
      table(
        [
          ["this", ""],
          ["breaks", ""],
          ["badly", ""],
        ],
        config
      )
    ).to.be.deep.equal(
      `
╔════════╤══╗
║ this   │  ║
╟────────┼──╢
║ breaks │  ║
╟────────┼──╢
║ badly  │  ║
╚════════╧══╝
`.trimLeft()
    );
  });
});

The error message appears to come from this function: https://github.com/gajus/table/blob/05533919abc14904d9639d33923d0aa6824e4ae6/src/wrapWord.ts#L4-L35

I haven't spent a lot of time with this codebase but could we Math.max the right side of the quantifier so it's at least as large as the left-side?

e.g.

-  const re = new RegExp('(^.{1,' + String(size) + '}(\\s+|$))|(^.{1,' + String(size - 1) + '}(\\\\|/|_|\\.|,|;|-))');
+  const re = new RegExp('(^.{1,' + String(Math.max(size, 1)) + '}(\\s+|$))|(^.{1,' + String(Math.max(size - 1, 1)) + '}(\\\\|/|_|\\.|,|;|-))');  

Or does it make sense to immediately bail while returning an empty list of chunks?

If you're happy with either of these fixes, or have your own suggestion, I can send you a PR.

nam-hle commented 2 years ago

Yes, please. Any suggestions are welcome :)