Phase 6: annotation authoring#32
Merged
Merged
Conversation
11 new exports: two lifecycle bookends + nine per-attribute
setters that close the create / mutate / delete surface for
annotations.
Lifecycle:
pdf_annot_new(page, subtype, bounds = NULL)
pdf_annot_delete(annot)
Setters mirroring every pdf_annot_* reader:
pdf_annot_set_bounds(annot, bounds) FPDFAnnot_SetRect
pdf_annot_set_color(annot, color, ...) FPDFAnnot_SetColor
pdf_annot_set_interior_color(annot, ...) FPDFAnnot_SetColor
(InteriorColor)
pdf_annot_set_flags(annot, flags) FPDFAnnot_SetFlags
pdf_annot_set_contents(annot, text) FPDFAnnot_SetStringValue
pdf_annot_set_title(annot, text) FPDFAnnot_SetStringValue
pdf_annot_set_subject(annot, text) FPDFAnnot_SetStringValue
pdf_annot_set_dict_value(annot, key, text) FPDFAnnot_SetStringValue
pdf_annot_append_quad(annot, quad) FPDFAnnot_AppendAttachmentPoints
Conventions (ADR-018):
* Color setters: composite — accept either a full `color` vector
or per-channel `red`/`green`/`blue`/`alpha` overrides; auto-
detect 0-255 vs 0-1 input form. A small units fix on the
partial-overlay path: the existing pdf_annot_color reader
returns 0..1 doubles, so when the setter reads the current
color for an overlay it scales back to 0..255 before merging
the user's 0..255 channels — without that step the unchanged
channels would clamp to near-zero.
* Flag setter: accepts either a raw integer bitmask or a named
logical matching the names that pdf_annot_flags_decoded()
returns; rejects unknown bit names. A new internal helper
`pdfium_annot_flag_encode()` is the inverse of the existing
`annot_flag_decode`.
* pdf_annot_delete: removes via FPDFPage_RemoveAnnot and then
R_ClearExternalPtr() so the handle reports closed via the
existing is_open() chain. The annot's own finalizer
(FPDFPage_CloseAnnot) becomes a no-op since the address is now
NULL — belt-and-braces against PDFium destroying the C++
object during RemoveAnnot.
* pdf_annot_new: wraps FPDFPage_CreateAnnot; supports the 13
subtypes PDFium accepts (circle, fileattachment, freetext,
highlight, ink, link, popup, square, squiggly, stamp,
strikeout, text, underline). Optional `bounds` argument calls
FPDFAnnot_SetRect immediately.
Test coverage: 62 new assertions covering round-trips for every
setter (read back through the corresponding pdf_annot_* getter),
0-255 / 0-1 color auto-detect, named-logical flag encoding,
non-ASCII UTF-8 contents, partial-overlay correctness, read-only
doc rejection, and closed-page rejection. 1949 R assertions
pass; 100% R coverage; no lints.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
R CMD check --as-cran flagged `pdf_annot_delete.Rd` as having an Rd
\link{} target missing a package anchor — `is_open()` is an
internal helper, not exported, so it has no Rd page to link to.
Replace the bracketed cross-reference with a plain \code{} so the
function name still renders as code but doesn't generate a broken
link.
Found by the R-CMD-check matrix on PR #32 (ubuntu / macos / windows
all flagged the same WARNING).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
billdenney
added a commit
that referenced
this pull request
May 21, 2026
CI flagged R CMD check WARNING on every platform:
pdf_doc_open_url.Rd: print.pdfium_doc
Please provide package anchors for all Rd \link{} targets not in
the package itself and the base packages.
The pdf_doc_open_url docstring had `[print()][print.pdfium_doc]` —
markdown for "render `print()` as link to topic `print.pdfium_doc`".
But `print.pdfium_doc` is an internal S3 method without its own Rd
page, so the link can't resolve.
Two changes:
1. Replace the bracketed cross-reference with plain `print()`
inline code so the function name still renders as code but
doesn't generate a broken link. Mirrors the same fix pattern
used on PR #32's `[is_open()]` issue.
2. New pre-commit hook `rd-xref-check` (entry:
tools/check-rd-xrefs.R) that runs the same internal R function
`R CMD check` uses for its cross-reference step
(tools:::.check_Rd_xrefs). Catches this class of WARNING on the
developer machine before push.
The script needs nothing more than the source tree (no install,
no compile, no C++ build), so it's cheap enough to run on
every commit — listed under `repo: local` next to the existing
`pkgdown-reference-check` hook, with the same diagnostic-then-
exit-1 pattern. Trigger files: any change to R/*.R, man/*.Rd,
or DESCRIPTION.
Manual verification: injecting the original broken link
reproduces the CI failure shape:
[check-rd-xrefs] pdf_doc_open_url.Rd: unresolved \link{}
target 'print.pdfium_doc'
Restoring the fix exits 0.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
11 new exports closing the create / mutate / delete loop for annotations.
pdf_annot_new(page, subtype, bounds = NULL)FPDFPage_CreateAnnot(+ optionalFPDFAnnot_SetRect)pdf_annot_delete(annot)FPDFPage_RemoveAnnot+ R-side handle invalidationpdf_annot_set_boundsFPDFAnnot_SetRectpdf_annot_set_colorFPDFAnnot_SetColor(Color)pdf_annot_set_interior_colorFPDFAnnot_SetColor(InteriorColor)pdf_annot_set_flagsFPDFAnnot_SetFlagspdf_annot_set_contentsFPDFAnnot_SetStringValue("Contents", …)pdf_annot_set_titleFPDFAnnot_SetStringValue("T", …)pdf_annot_set_subjectFPDFAnnot_SetStringValue("Subj", …)pdf_annot_set_dict_valueFPDFAnnot_SetStringValue(key, …)pdf_annot_append_quadFPDFAnnot_AppendAttachmentPointsConventions (ADR-018)
colorvector or per-channel overrides; auto-detect 0-255 ints vs 0-1 doubles. Bug fix on the partial-overlay path:pdf_annot_color()returns 0..1, so when the setter reads the current color for an overlay it scales back to 0..255 before merging — without that step the unchanged channels would clamp to near-zero.pdf_annot_flags_decoded(). New internalpdfium_annot_flag_encode()is the inverse of the existing decoder.FPDFPage_RemoveAnnotandR_ClearExternalPtrs the handle so the existingis_open()chain reports closed for subsequent calls. The annot's own finalizer becomes a no-op (it's nullptr-tolerant).Subtypes supported by
pdf_annot_newPDFium accepts:
circle,fileattachment,freetext,highlight,ink,link,popup,square,squiggly,stamp,strikeout,text,underline. Other subtypes (e.g.widget,polygon,line) error from PDFium with a clear message.Carve-outs (deferred to future phases)
FPDFAnnot_SetAP+FPDFAnnot_AppendObject): a non-trivial workflow that overlaps with the page-object creators. Deferred — the existingpdf_annot_appearance()reader is enough to read AP streams and the writer side can come later.FPDFAnnot_SetAttachmentPoints(index, quad)(rewrite quad at index): only the append form ships. Reader returns the whole quad list; the natural authoring model is build-from-scratch + usepdf_annot_append_quadper quad.Test plan
devtools::test()— 1949 R assertions, 0 failures.lintr::lint_package()— no lints."日本語のテスト"round-trips).Stacking note
Branches off
claude/0.1.0-phase5-obj-creators(which in turn stacks on Phase 4). Please merge Phases 4 → 5 → 6 in order; each phase touches different source files so theNAMESPACE/RcppExportsmerges should be conflict-free.🤖 Generated with Claude Code