diff --git a/CLAUDE.md b/CLAUDE.md index 4cc724c..df704d3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -36,3 +36,10 @@ scripts/build.sh # pyinstaller binary build - Exact match for postprocess tests (we control the input) - Test PDFs generated at runtime via PyMuPDF Story API (no binaries in git) - Click `CliRunner` for CLI integration tests + +## Fixing GitHub Issues + +1. Create a fix branch from `main` +2. Make the changes +3. Run build (`scripts/build.sh`) and tests (`pytest -v`) +4. Open a PR on that branch diff --git a/pdf2md/converter.py b/pdf2md/converter.py index 0073597..704ec8a 100644 --- a/pdf2md/converter.py +++ b/pdf2md/converter.py @@ -1,7 +1,10 @@ from __future__ import annotations import logging +import shutil +import tempfile from dataclasses import dataclass +from pathlib import Path import pymupdf4llm @@ -52,7 +55,21 @@ def convert_pdf(path: str, options: ConversionOptions) -> str: if options.pages is not None: kwargs["pages"] = options.pages - if options.image_path: + # Work around pymupdf4llm replacing spaces with underscores in image + # paths (utils.md_path sanitizes the save path but the directory is + # created with the original name, causing a file-not-found error). + # When the image path contains spaces, write to a temp directory first + # and relocate the images afterwards. + real_image_path = options.image_path + temp_image_dir: str | None = None + + if options.image_path and " " in options.image_path: + temp_image_dir = tempfile.mkdtemp(prefix="pdf2md_") + kwargs["image_path"] = temp_image_dir + logger.debug( + "Using temp image dir %s (real: %s)", temp_image_dir, real_image_path + ) + elif options.image_path: kwargs["image_path"] = options.image_path logger.debug("Converting %s with options: %s", path, kwargs) @@ -60,9 +77,44 @@ def convert_pdf(path: str, options: ConversionOptions) -> str: try: result = pymupdf4llm.to_markdown(path, **kwargs) except Exception as exc: + if temp_image_dir: + shutil.rmtree(temp_image_dir, ignore_errors=True) raise Pdf2mdError(f"Conversion failed: {exc}") from exc if not isinstance(result, str): + if temp_image_dir: + shutil.rmtree(temp_image_dir, ignore_errors=True) raise Pdf2mdError("Unexpected output type from pymupdf4llm") + if temp_image_dir: + try: + result = _relocate_images(temp_image_dir, real_image_path, result) + except Exception: + shutil.rmtree(temp_image_dir, ignore_errors=True) + raise + return result + + +def _relocate_images(src_dir: str, dst_dir: str, markdown: str) -> str: + """Move images from a temp directory to the real destination. + + Also rewrites image references in the markdown text so they point to + the correct location. + """ + src = Path(src_dir) + dst = Path(dst_dir) + dst.mkdir(parents=True, exist_ok=True) + + for item in src.iterdir(): + if item.is_file(): + shutil.move(str(item), str(dst / item.name)) + + # pymupdf4llm emits absolute posix paths in the markdown — replace the + # temp dir prefix with the real destination so image links resolve. + src_abs = src.resolve().as_posix() + dst_abs = dst.resolve().as_posix() + markdown = markdown.replace(src_abs, dst_abs) + + shutil.rmtree(src_dir, ignore_errors=True) + return markdown diff --git a/tests/conftest.py b/tests/conftest.py index ddf2b27..f07867e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -36,6 +36,11 @@ def with_lists_pdf() -> str: return str(FIXTURES_DIR / "with_lists.pdf") +@pytest.fixture +def with_image_pdf() -> str: + return str(FIXTURES_DIR / "with_image.pdf") + + @pytest.fixture def cli_runner() -> CliRunner: return CliRunner() diff --git a/tests/fixtures/generate_fixtures.py b/tests/fixtures/generate_fixtures.py index cda1261..af3ac26 100644 --- a/tests/fixtures/generate_fixtures.py +++ b/tests/fixtures/generate_fixtures.py @@ -98,12 +98,32 @@ def generate_with_lists() -> None: _write_story_pdf(html, FIXTURES_DIR / "with_lists.pdf") +def generate_with_image() -> None: + """Generate a PDF containing an embedded image.""" + output = FIXTURES_DIR / "with_image.pdf" + doc = pymupdf.open() + page = doc.new_page(width=612, height=792) + + # Create a colored pixmap (RGBA: 4 components for alpha=1) + rect = pymupdf.Rect(100, 200, 300, 400) + pix = pymupdf.Pixmap(pymupdf.csRGB, pymupdf.IRect(rect), 1) + pix.set_rect(pymupdf.IRect(rect), (255, 0, 0, 255)) + page.insert_image(rect, pixmap=pix) + + # Add some text + page.insert_text((100, 100), "Document With Image", fontsize=20) + + doc.save(str(output)) + doc.close() + + def generate_all() -> None: """Generate all test PDF fixtures.""" generate_simple_text() generate_with_tables() generate_multi_page() generate_with_lists() + generate_with_image() if __name__ == "__main__": diff --git a/tests/test_cli.py b/tests/test_cli.py index 494e4eb..fda7664 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -73,3 +73,17 @@ def test_verbose(self, cli_runner: CliRunner, simple_text_pdf: str): def test_keep_headers_footers(self, cli_runner: CliRunner, simple_text_pdf: str): result = cli_runner.invoke(main, [simple_text_pdf, "--keep-headers-footers"]) assert result.exit_code == 0 + + def test_image_dir_with_spaces( + self, cli_runner: CliRunner, with_image_pdf: str, tmp_path + ): + """Regression: --image-dir with spaces should not fail (GH-1).""" + image_dir = str(tmp_path / "path with spaces" / "imgs") + output_file = str(tmp_path / "out.md") + result = cli_runner.invoke( + main, + [with_image_pdf, "--images", "--image-dir", image_dir, "-o", output_file], + ) + assert result.exit_code == 0, result.output + written = list(Path(image_dir).glob("*.png")) + assert len(written) > 0 diff --git a/tests/test_converter.py b/tests/test_converter.py index cab7f8b..5b9bcee 100644 --- a/tests/test_converter.py +++ b/tests/test_converter.py @@ -1,5 +1,7 @@ from __future__ import annotations +from pathlib import Path + from pdf2md.converter import ConversionOptions, convert_pdf @@ -49,3 +51,32 @@ def test_returns_string(self, simple_text_pdf): result = convert_pdf(simple_text_pdf, ConversionOptions()) assert isinstance(result, str) assert len(result) > 0 + + +class TestImageDirWithSpaces: + def test_image_dir_with_spaces(self, with_image_pdf, tmp_path): + """Regression test: spaces in image-dir path should not cause errors.""" + image_dir = str(tmp_path / "dir with spaces" / "images here") + options = ConversionOptions( + write_images=True, + ignore_images=False, + image_path=image_dir, + ) + result = convert_pdf(with_image_pdf, options) + # Images should be written to the directory with spaces + written = list(Path(image_dir).glob("*.png")) + assert len(written) > 0, "No images written to directory with spaces" + # Markdown should reference the real path, not a temp path + assert "pdf2md_" not in result + + def test_image_dir_without_spaces(self, with_image_pdf, tmp_path): + """Image extraction works normally for paths without spaces.""" + image_dir = str(tmp_path / "normal_images") + options = ConversionOptions( + write_images=True, + ignore_images=False, + image_path=image_dir, + ) + convert_pdf(with_image_pdf, options) + written = list(Path(image_dir).glob("*.png")) + assert len(written) > 0, "No images written to directory"