Skip to content

fix: follow-ups to #437 parseBuffer fix: - #438

Merged
modesty merged 1 commit into
masterfrom
fix/pr437-review-followups
Nov 6, 2026
Merged

modesty merged 1 commit into
masterfrom
fix/pr437-review-followups

Conversation

@modesty

@modesty modesty commented Nov 6, 2026

Copy link
Copy Markdown
Owner
  • parseBuffer: accept any non-empty ArrayBufferView, including DataView, which the old check rejected. Inputs that are not views still throw.
  • loadPDF/parseBuffer: use so verbosity 0 (ERRORS) is honored instead of falling back to WARNINGS.
  • Stream: check explicitly, so a sub-stream at offset 0 with length 0 is empty instead of the whole buffer.
  • Field tab indexes (TI): number them per document, from 0, in page order. The counter was shared by the whole process, so TI depended on how many documents had been parsed before, including concurrently.
  • types: parseBuffer(pdfBuffer: ArrayBufferView) in .d.ts and .d.cts, matching the typed-array support the implementation already had.
  • JSDoc: parseBuffer and #startParsingPDF describe the accepted inputs and the throw.

Tests:

  • _test_parseBuffer_offset: add a DataView case; force Buffer.from to use the shared pool at a non-zero offset (before, the test passed even with the bug on Node 22's 8 KiB pool); compare full output now that TI is deterministic; the helper uses once() and always calls destroy(); add unit tests for Stream sub-streams against base/core/stream.js.
  • _test_errors: inputs that are not views and an empty DataView throw; verbosity 0 hides engine warnings for both parseBuffer and loadPDF.
  • _test_testMultipleDataPDF: TI starts at 0 for repeated, same-parser and concurrent parses.

Docs: README API reference documents parseBuffer inputs, the
verbosity levels and how TI is numbered; unit suite counts updated.

Behavior change: when several PDFs are parsed in one process (e.g. the
CLI on a directory), TI now starts at 0 for each file instead of
continuing from the previous one. Corpus check: across 1064 test:forms
output files, the only differences from master are TI values.

  - parseBuffer: accept any non-empty ArrayBufferView, including DataView,
    which the old check rejected. Inputs that are not views still throw.
  - loadPDF/parseBuffer: use  so verbosity 0 (ERRORS)
    is honored instead of falling back to WARNINGS.
  - Stream: check  explicitly, so a sub-stream at
    offset 0 with length 0 is empty instead of the whole buffer.
  - Field tab indexes (TI): number them per document, from 0, in page
    order. The counter was shared by the whole process, so TI depended on
    how many documents had been parsed before, including concurrently.
  - types: parseBuffer(pdfBuffer: ArrayBufferView) in .d.ts and .d.cts,
    matching the typed-array support the implementation already had.
  - JSDoc: parseBuffer and #startParsingPDF describe the accepted inputs
    and the throw.

  Tests:
  - _test_parseBuffer_offset: add a DataView case; force Buffer.from to
    use the shared pool at a non-zero offset (before, the test passed even
    with the bug on Node 22's 8 KiB pool); compare full output now that TI
    is deterministic; the helper uses once() and always calls destroy();
    add unit tests for Stream sub-streams against base/core/stream.js.
  - _test_errors: inputs that are not views and an empty DataView throw;
    verbosity 0 hides engine warnings for both parseBuffer and loadPDF.
  - _test_testMultipleDataPDF: TI starts at 0 for repeated, same-parser
    and concurrent parses.

  Docs: README API reference documents parseBuffer inputs, the
  verbosity levels and how TI is numbered; unit suite counts updated.

  Behavior change: when several PDFs are parsed in one process (e.g. the
  CLI on a directory), TI now starts at 0 for each file instead of
  continuing from the previous one. Corpus check: across 1064 test:forms
  output files, the only differences from master are TI values.
@modesty
modesty merged commit ebe46e5 into master Nov 6, 2026
2 checks passed
@modesty
modesty deleted the fix/pr437-review-followups branch November 6, 2026 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant