Repository navigation
fix(serializer): keep % and # in LaTeX includegraphics paths - #970
shashank11yadav wants to merge 1 commit into
Conversation
With image_mode=REFERENCED the image URI was copied as is into \includegraphics. A "%" in the path starts a TeX comment and swallows the closing brace, and a "#" gives "Illegal parameter number", so the generated .tex file no longer compiles. Backslash escapes do not work in a graphicx file name, so when the path contains "%" or "#" the line is wrapped in a group that makes both characters ordinary. Paths without them are emitted unchanged. Fixes docling-project#969 Signed-off-by: Shashank Yadav <65058495+shashank11yadav@users.noreply.github.com>
|
✅ DCO Check Passed Thanks @shashank11yadav, all your commits are properly signed off. 🎉 |
Merge Protections🔴 1 of 2 protections blocking · waiting on 👀 reviews
🔴 Require two reviewer for test updatesWaiting for
This rule is failing.When test data is updated, we require two reviewers
Show 1 satisfied protection🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
AI triage (advisory)Checked commit This PR wraps Topic labels added: CI safety: no concern found. A maintainer must still approve CI. The |
There was a problem hiding this comment.
AI first review (advisory): no blocker found
This review covers the full diff at 13e1f26b7d. It read the code and did not run it.
I found no defect that blocks the merge. I checked that _serialize_image_part is called only from the picture serializer, so the new \begingroup … \endgroup line sits directly in the figure environment and not inside a macro argument. Paths without % or # are emitted as before, and the new test checks the exact emitted line for the %, # and plain cases.
A maintainer makes the merge decision. This review never approves the pull request.
With
image_mode=ImageRefMode.REFERENCED, the LaTeX serializer copied the image URI into\includegraphics[width=\linewidth]{...}unchanged. A%in the path starts a TeX comment, which swallows the closing brace. A#gives "Illegal parameter number". The generated.texfile did not compile.When the image path contains
%or#, the\includegraphicsline is now wrapped in\begingroup ... \endgroup, with\catcodeset to 12 (other) for%and#. This differs from the\%escape suggested in #969: with pdflatex,\%or\#in an\includegraphicsfile name is an error (Missing \endcsname inserted). Paths without%or#are emitted exactly as before.Scope:
%and#are handled. Braces and backslashes in file names are unchanged and left for a follow-up.\fbox{...}),%and#are already read by then and the line still breaks.Testing:
test_latex_referenced_image_pathintests/test_latex_serialization.pychecks the exact emitted line for a path with%, a path with#, and a plain path. The%and#cases fail without this change. The plain path case passes before and after.ruff check,ruff format --checkandty checkpass.pytest testsrun: 1268 passed, 12 skipped, 1 xfailed. Five tokenizer-download test modules and two network tests were not run.%, one with#and a plain one, with a real image at each path. With this change all three compile and load the image; onmainthe%and#cases fail. (nicefracandulemwere dropped from the preamble for that run because they weren't installed locally; they don't affect images.) XeLaTeX and LuaLaTeX were not tested.Fixes #969