Skip to content

Improve binary file check to better handle PDFs - #150

Merged
mmrwoods merged 1 commit into
mainfrom
fix_pdf_binary_check
Oct 3, 2026
Merged

mmrwoods merged 1 commit into
mainfrom
fix_pdf_binary_check

Conversation

@mmrwoods

@mmrwoods mmrwoods commented Oct 3, 2026

Copy link
Copy Markdown
Member

PDF files can have a very long text header before the first NUL byte

The check now reads the first 2KB rather than 128 bytes, and uses the
built-in index() function rather than looping which is very fast. That
still didn't catch all PDFs, even increasing it to 8000 bytes like git
diff didn't catch all PDFs, so also check for %PDF- signature in file.

The increased number of bytes to check remains even with the %PDF-
signature check because it costs very little (page size is min 4KB),
and will potentially catch other binary files with long text headers.

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 review overview

🟢 Approval recommended

The implementation is sound; only a minor comment wording issue remains.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
What changed in this PR

Improves binary detection for PDFs and files with long textual prefixes.

Changes:

  • Expands NUL-byte scanning from 128 bytes to 2 KB.
  • Detects the %PDF- signature directly.
  • Uses index() instead of a byte loop.
File Description
autoload/​fuzzbox/​internal/​previewer.vim Enhances binary and PDF detection.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread autoload/fuzzbox/internal/previewer.vim Outdated
PDF files can have a very long text header before the first NUL byte

The check now reads the first 2KB rather than 128 bytes, and uses the
built-in index() function rather than looping which is very fast. That
still didn't catch all PDFs, even increasing it to 8000 bytes like git
diff didn't catch all PDFs, so also check for %PDF- signature in file.

The increased number of bytes to check remains even with the %PDF-
signature check because it costs very little (page size is min 4KB),
and will potentially catch other binary files with long text headers.
@mmrwoods
mmrwoods force-pushed the fix_pdf_binary_check branch from 997bed1 to 9073fae Compare October 3, 2026 12:00
@mmrwoods
mmrwoods merged commit 6cdd7c0 into main Oct 3, 2026
2 checks passed
@mmrwoods
mmrwoods deleted the fix_pdf_binary_check branch October 3, 2026 12:00
@mmrwoods
mmrwoods restored the fix_pdf_binary_check branch October 3, 2026 12:02
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