Repository navigation
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
Closed
erkurtharun wants to merge 1 commit into
erkurtharun wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🔍 Devin Review: 1 flag
Not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
…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.
erkurtharun
force-pushed
the
bcmaps-dir-env
branch
from
October 15, 2026 15:52
2568383 to
c808c08
Compare
There was a problem hiding this comment.
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
Author
|
Closing this: I no longer use pdf-inspector and can't follow it through. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PDF_INSPECTOR_BCMAPS_DIRswaps 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.