Skip to content

feat(tounicode)!: read the built-in CMaps only from the binary, not PDF_INSPECTOR_BCMAPS_DIR - #632

Closed
erkurtharun wants to merge 1 commit into
firecrawl:mainfrom
erkurtharun:bcmaps-dir-env
Closed

erkurtharun wants to merge 1 commit into
firecrawl:mainfrom
erkurtharun:bcmaps-dir-env

Conversation

@erkurtharun

@erkurtharun erkurtharun commented Oct 15, 2026 •

Copy link
Copy Markdown

PDF_INSPECTOR_BCMAPS_DIR swaps the CMaps compiled into the binary (#570) for files on disk, so the same PDF converts differently depending on a process-wide variable that the library reads on every lookup and that the API offers no way to opt out of. This removes it; the built-in CMaps come only from the binary.

Breaking for anyone who sets the variable: replacing a CMap now takes a rebuild. Noted in the CHANGELOG under Changed.

Tested with a new unit test that reruns the test binary with the variable pointing at a directory holding a different copy of one CMap and checks that the embedded bytes are still read; it fails with the override restored.

Copilot AI balanced review requested due to automatic review settings October 15, 2026 12:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Devin Review: 1 flag

Not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)

Devin Review

cubic-dev-ai[bot]

This comment was marked as resolved.

…DF_INSPECTOR_BCMAPS_DIR

PDF_INSPECTOR_BCMAPS_DIR swaps the CMaps compiled into the binary for
files on disk. The same PDF then converts differently depending on a
process-wide variable that the library reads on every built-in CMap
lookup and that the API offers no way to opt out of. The built-in CMaps
are now read only from the binary.

Anyone who sets the variable loses the override; replacing a CMap takes
a rebuild.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 1 file (changes from recent commits).

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Shadow auto-approve: would require human review. Removes the PDF_INSPECTOR_BCMAPS_DIR override so all CMap data comes from compiled-in assets. This is a deliberate breaking change to the library's runtime configuration and to how users replace CMaps.

View guided diff | Re-trigger cubic

@erkurtharun

Copy link
Copy Markdown
Author

Closing this: I no longer use pdf-inspector and can't follow it through.

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.

2 participants