diff --git a/CLAUDE.md b/CLAUDE.md index a8c9d3680..0aeb62bf7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -56,8 +56,9 @@ Reference `git log 253dbc87 --format=%B -n1` for the canonical shape: scope, des Concretely: - Agents **author** the UAT script (it's part of the deliverable per §6.3). -- Agents **may run** the UAT — once, after the §7 trinity is green — to verify the script doesn't crash, exits non-zero on failure, and actually asserts what it claims to assert. This is QA on the test itself, not acceptance of the feature. +- Agents **may run** the UAT — after the §7 trinity is green — to verify the script doesn't crash, exits non-zero on failure, and actually asserts what it claims to assert. This is QA on the test itself, not acceptance of the feature. - Agents **must not report or imply UAT signoff** in commit messages, PR bodies, or summaries. Acceptable: *"UAT script runs clean — pending maintainer visual signoff in PowerPoint/Keynote."* Not acceptable: *"UAT: PASS — round-trip confirmed."* +- UAT files should be stored under the `./uat` subdirectory. - The §7 reporting trinity below (pytest + ruff + behave) is the agent's full self-verification surface. UAT execution output may be included as an attachment but it is **not** the fourth gate. - Round-trip / behavioral evidence within agent-runnable scope still goes through **pytest integration tests** that exercise save+reopen. If pytest can fully cover it, the UAT is just a maintainer convenience; if pytest can't, UAT becomes the maintainer's primary acceptance path. - If unsure whether the maintainer has delegated signoff for a particular case, the agent **stops and asks**. diff --git a/features/sld-comments.feature b/features/sld-comments.feature new file mode 100644 index 000000000..a5f7d23bf --- /dev/null +++ b/features/sld-comments.feature @@ -0,0 +1,35 @@ +Feature: Modern threaded comments on a slide (issue #25) + In order to round-trip review feedback in a presentation + As a developer using python-pptx + I need to add, read, reply to, and remove threaded comments on a slide + + Scenario: Add a threaded comment and read it back after save/reopen + Given a blank slide with no comments + When I add a comment "Looks great" by author "Ada" + Then the slide has 1 comment + And after save and reopen the first comment text is "Looks great" by author "Ada" + + Scenario: Reply to a comment threads under it and round-trips + Given a blank slide with no comments + When I add a comment "Question?" by author "Ada" + And I reply "Here is the answer" by author "Babbage" + Then after save and reopen the first comment has 1 reply with text "Here is the answer" + + Scenario: Removing the last comment leaves a valid reopenable file + Given a blank slide with no comments + When I add a comment "Temporary" by author "Ada" + And I remove that comment + Then the slide has 0 comments + And after save and reopen the slide has 0 comments + + Scenario: Resolving a threaded comment round-trips + Given a blank slide with no comments + When I add a comment "Please review" by author "Ada" + And I resolve that comment + Then after save and reopen the first comment is resolved + + Scenario: A legacy comment and a modern comment coexist on round-trip + Given a slide carrying a pre-existing legacy comment + When I add a modern comment "Modern note" by author "Mary" + Then after save and reopen both the legacy and modern comments are present + And after save and reopen the legacy author ids are intact diff --git a/features/steps/comments.py b/features/steps/comments.py new file mode 100644 index 000000000..23b80107b --- /dev/null +++ b/features/steps/comments.py @@ -0,0 +1,221 @@ +"""Gherkin step implementations for modern threaded comments (issue #25).""" + +from __future__ import annotations + +import io + +from behave import given, then, when + +from pptx import Presentation + + +# given =================================================== + + +@given("a blank slide with no comments") +def given_blank_slide_no_comments(context): + prs = Presentation() + slide = prs.slides.add_slide(prs.slide_layouts[6]) + context.prs = prs + context.slide = slide + assert len(slide.comments) == 0 + + +# when ==================================================== + + +@when('I add a comment "{text}" by author "{author}"') +def when_add_comment(context, text, author): + context.comment = context.slide.comments.add(text, author) + + +@when('I reply "{text}" by author "{author}"') +def when_add_reply(context, text, author): + context.comment.replies.add(text, author) + + +@when("I remove that comment") +def when_remove_comment(context): + context.slide.comments.remove(context.comment) + + +# then ==================================================== + + +@then("the slide has {count:d} comment") +@then("the slide has {count:d} comments") +def then_slide_has_n_comments(context, count): + assert len(context.slide.comments) == count, "expected %d comments, got %d" % ( + count, + len(context.slide.comments), + ) + + +@then('after save and reopen the first comment text is "{text}" by author "{author}"') +def then_reopen_first_comment(context, text, author): + stream = io.BytesIO() + context.prs.save(stream) + stream.seek(0) + comment = list(Presentation(stream).slides[0].comments)[0] + assert comment.text == text, "text was %r" % comment.text + assert comment.author == author, "author was %r" % comment.author + + +@then('after save and reopen the first comment has {count:d} reply with text "{text}"') +def then_reopen_reply(context, count, text): + stream = io.BytesIO() + context.prs.save(stream) + stream.seek(0) + comment = list(Presentation(stream).slides[0].comments)[0] + replies = list(comment.replies) + assert len(replies) == count, "expected %d replies, got %d" % (count, len(replies)) + assert replies[0].text == text, "reply text was %r" % replies[0].text + + +@then("after save and reopen the slide has {count:d} comments") +def then_reopen_comment_count(context, count): + stream = io.BytesIO() + context.prs.save(stream) + stream.seek(0) + reopened = Presentation(stream).slides[0] + assert len(reopened.comments) == count, "expected %d comments after reopen, got %d" % ( + count, + len(reopened.comments), + ) + + +# -- Wave 3: SF6 resolve + SF8 legacy/modern coexistence ------------------------ + + +@when("I resolve that comment") +def when_resolve_comment(context): + context.comment.resolve() + + +@then("after save and reopen the first comment is resolved") +def then_reopen_first_comment_resolved(context): + stream = io.BytesIO() + context.prs.save(stream) + stream.seek(0) + comment = list(Presentation(stream).slides[0].comments)[0] + assert comment.resolved is True, "comment.resolved was %r" % comment.resolved + + +def _build_legacy_plus_modern(prs0): + """Inject a legacy + author + slide rel into a saved package and + return the reopened Presentation (modern not yet added).""" + import zipfile + + base = io.BytesIO() + prs0.save(base) + base.seek(0) + + legacy_authors = ( + '\n' + '' + '' + ) + legacy_comments = ( + '\n' + '' + '' + 'Legacy feedback' + "" + ) + src = zipfile.ZipFile(base, "r") + names = src.namelist() + ct = ( + src.read("[Content_Types].xml") + .decode("utf-8") + .replace( + "", + '' + '', + ) + ) + pres_rels = ( + src.read("ppt/_rels/presentation.xml.rels") + .decode("utf-8") + .replace( + "", + '' + "", + ) + ) + slide_rels = ( + src.read("ppt/slides/_rels/slide1.xml.rels") + .decode("utf-8") + .replace( + "", + '' + "", + ) + ) + patched = { + "[Content_Types].xml": ct, + "ppt/_rels/presentation.xml.rels": pres_rels, + "ppt/slides/_rels/slide1.xml.rels": slide_rels, + "ppt/commentAuthors.xml": legacy_authors, + "ppt/comments/comment1.xml": legacy_comments, + } + out = io.BytesIO() + dst = zipfile.ZipFile(out, "w", zipfile.ZIP_DEFLATED) + for name in names: + dst.writestr(name, patched.get(name, src.read(name))) + for name, data in patched.items(): + if name not in names: + dst.writestr(name, data) + src.close() + dst.close() + out.seek(0) + return Presentation(out) + + +@given("a slide carrying a pre-existing legacy comment") +def given_legacy_comment_slide(context): + prs0 = Presentation() + prs0.slides.add_slide(prs0.slide_layouts[6]) + context.prs = _build_legacy_plus_modern(prs0) + context.slide = context.prs.slides[0] + assert any(c.is_legacy for c in context.slide.comments) + + +@when('I add a modern comment "{text}" by author "{author}"') +def when_add_modern_comment(context, text, author): + context.comment = context.slide.comments.add(text, author) + + +@then("after save and reopen both the legacy and modern comments are present") +def then_reopen_both_families(context): + stream = io.BytesIO() + context.prs.save(stream) + stream.seek(0) + comments = list(Presentation(stream).slides[0].comments) + texts = sorted(c.text for c in comments) + assert texts == ["Legacy feedback", "Modern note"], "texts were %r" % texts + + +@then("after save and reopen the legacy author ids are intact") +def then_reopen_legacy_author_ids(context): + import zipfile + + stream = io.BytesIO() + context.prs.save(stream) + stream.seek(0) + zf = zipfile.ZipFile(stream, "r") + legacy_xml = zf.read("ppt/comments/comment1.xml").decode("utf-8") + authors_xml = zf.read("ppt/commentAuthors.xml").decode("utf-8") + zf.close() + assert 'authorId="0"' in legacy_xml, "legacy authorId mangled: %r" % legacy_xml + assert 'name="Legacy Larry"' in authors_xml, "legacy author lost: %r" % authors_xml diff --git a/src/pptx/__init__.py b/src/pptx/__init__.py index dc53a39d3..df16aaadc 100644 --- a/src/pptx/__init__.py +++ b/src/pptx/__init__.py @@ -10,6 +10,12 @@ from pptx.opc.constants import CONTENT_TYPE as CT from pptx.opc.package import PartFactory from pptx.parts.chart import ChartPart +from pptx.parts.comments import ( + AuthorsPart, + CommentAuthorsPart, + CommentsPart, + ModernCommentsPart, +) from pptx.parts.coreprops import CorePropertiesPart from pptx.parts.custom_properties import CustomPropertiesPart from pptx.parts.custom_xml import CustomXmlPropertiesPart @@ -41,6 +47,10 @@ CT.PML_TEMPLATE_MAIN: PresentationPart, CT.PML_SLIDESHOW_MAIN: PresentationPart, CT.OPC_CORE_PROPERTIES: CorePropertiesPart, + CT.PML_COMMENT_AUTHORS: CommentAuthorsPart, + CT.PML_COMMENTS: CommentsPart, + CT.PML_THREADED_COMMENTS: ModernCommentsPart, + CT.PML_AUTHORS: AuthorsPart, CT.OFC_CUSTOM_PROPERTIES: CustomPropertiesPart, CT.OFC_CUSTOM_XML_PROPERTIES: CustomXmlPropertiesPart, # NOTE: CT.XML is intentionally NOT mapped to CustomXmlPart — see @@ -80,6 +90,10 @@ del ( ChartPart, + AuthorsPart, + CommentAuthorsPart, + CommentsPart, + ModernCommentsPart, CorePropertiesPart, CustomPropertiesPart, CustomXmlPropertiesPart, diff --git a/src/pptx/comments.py b/src/pptx/comments.py new file mode 100644 index 000000000..6d6cc9dc3 --- /dev/null +++ b/src/pptx/comments.py @@ -0,0 +1,415 @@ +"""Public proxy objects for modern threaded comments (issue #25, Wave 2). + +These are the API surface a caller actually touches: + +* :class:`Comments` — the per-slide collection returned by + ``Slide.comments``. Iterable in document order, ``len()``-able, with + ``add(text, author, anchor=None)`` and ``remove(comment)``. +* :class:`Comment` — one threaded comment. Exposes ``text``, ``author`` + (resolved authorId→name), ``created_at`` (tz-aware ``datetime`` or + ``None``), ``anchor_position`` ((x, y) |Length| pair or ``None``), and + a ``replies`` collection. +* :class:`CommentReplies` / :class:`CommentReply` — the reply thread under + a comment and one reply in it. + +The collection lazily creates+relates the per-slide |ModernCommentsPart| +on first ``add`` (mirroring ``SlidePart.notes_slide``), and get-or-adds +the presentation-level |CommentAuthorsPart| author by name with no +duplicates (ISC-5). Removing the last comment leaves a valid empty +```` and intact slide rels (ISC-21 anti-criterion). + +No upstream `scanny/python-pptx` prior art — built from the OOXML / +MS-OOXML schemas and this fork's part conventions. +""" + +from __future__ import annotations + +import uuid +from datetime import datetime +from typing import TYPE_CHECKING, Iterator + +from pptx.util import Emu, Length + +if TYPE_CHECKING: + from pptx.oxml.comments import ( + CT_Comment, + CT_ThreadedComment, + CT_ThreadedCommentReply, + ) + from pptx.slide import Slide + +# Modern threaded-comment resolution marker. The MS 2018/8/main schema gives +# `` an optional ``status`` whose enumeration includes ``resolved``; +# PowerPoint stamps exactly this when a reviewer closes a thread. Absence (or +# any non-``resolved`` value) means the thread is still open. +_RESOLVED_STATUS = "resolved" + + +def _parse_created(raw: str | None) -> datetime | None: + """Parse an OOXML ``created`` timestamp into a tz-aware ``datetime``. + + PowerPoint writes ISO-8601 (commonly ``2026-05-16T12:00:00.000`` or + with a ``Z``/offset). Returns a timezone-aware ``datetime`` (assuming + UTC when no offset is present) or ``None`` when the value is missing + or unparseable — never raises, so a malformed file still reads. + """ + if not raw: + return None + text = raw.strip() + if text.endswith("Z"): + text = text[:-1] + "+00:00" + try: + dt = datetime.fromisoformat(text) + except ValueError: + return None + if dt.tzinfo is None: + from datetime import timezone + + dt = dt.replace(tzinfo=timezone.utc) + return dt + + +def _now_iso() -> str: + """Return the current UTC time as the ISO string PowerPoint expects. + + PowerPoint validates ``/@created`` as ``xsd:dateTime``; the + modern threaded-comment schema wants an explicit UTC designator. A + naive ``...sss`` value with no offset (the previous behavior) is one of + the things that triggered the issue-#25 repair dialog, so emit + ``YYYY-MM-DDThh:mm:ssZ`` (UTC, ``Z``-suffixed, second precision). + """ + from datetime import timezone + + return datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") + + +def _new_guid() -> str: + """Return a brace-wrapped uppercase GUID like PowerPoint emits.""" + return "{%s}" % str(uuid.uuid4()).upper() + + +class Comment: + """One comment on a slide — modern threaded *or* legacy. + + python-pptx only ever *writes* the modern (2018) ```` form, but + a deck authored elsewhere may carry pre-2018 legacy ```` comments. + Both are surfaced through the same proxy so callers iterating + ``slide.comments`` see one uniform collection (issue #25 Wave 3, SF8 / + ISC-44, ISC-67). A legacy-backed instance is read-only: it has no + resolution concept and no reply thread, and :meth:`resolve` raises. + :attr:`is_legacy` distinguishes the two. + """ + + def __init__(self, cm: "CT_ThreadedComment | CT_Comment", slide: "Slide"): + self._cm = cm + self._slide = slide + + def __eq__(self, other: object) -> bool: + return isinstance(other, Comment) and other._cm is self._cm + + def __ne__(self, other: object) -> bool: + return not self.__eq__(other) + + @property + def is_legacy(self) -> bool: + """`True` when this comment is a pre-2018 legacy ````. + + Legacy-backed comments are read-only proxies: :meth:`resolve` raises + and :attr:`replies` is always empty (the legacy schema has neither a + resolution flag nor a reply thread). + """ + from pptx.oxml.comments import CT_Comment as _CT_Comment + + return isinstance(self._cm, _CT_Comment) + + @property + def text(self) -> str: + """Plain-text body of this comment.""" + return self._cm.text + + @text.setter + def text(self, value: str) -> None: + if self.is_legacy: + raise TypeError( + "cannot edit a legacy comment — python-pptx only " + "writes modern threaded comments; legacy comments are " + "read-only (issue #25 SF8)" + ) + self._cm.text = value + + @property + def author(self) -> str | None: + """Display name of this comment's author, resolved authorId→name. + + Modern comments key the author by deterministic GUID; legacy + comments key by the integer ``authorId`` into the presentation + ````. Returns ``None`` when the id has no matching + author (a malformed or externally-edited file). + """ + if self.is_legacy: + return self._slide._resolve_legacy_author_name(self._cm.authorId) + return self._slide._resolve_author_name(self._cm.authorId) + + @property + def created_at(self) -> datetime | None: + """Creation timestamp as a tz-aware ``datetime``, or ``None``. + + Reads ``created`` for modern comments and ``dt`` for legacy ones. + """ + raw = self._cm.dt if self.is_legacy else self._cm.created + return _parse_created(raw) + + @property + def resolved(self) -> bool: + """`True` when this thread has been marked resolved. + + Defaults to ``False``. Legacy comments are *always* ``False`` — the + pre-2018 schema has no resolution concept (ISC-37). + """ + if self.is_legacy: + return False + return self._cm.status == _RESOLVED_STATUS + + def resolve(self) -> None: + """Mark this threaded comment's conversation resolved (ISC-33..35). + + Sets the modern ```` ``status="resolved"`` marker; the flag + round-trips through save/reopen and is read back by :attr:`resolved`. + + :raises TypeError: when called on a comment backed by a legacy + ```` element. **Design decision (ISC-37):** legacy raises + rather than being a silent no-op. The pre-2018 PresentationML + comment schema has no resolution attribute, so there is nowhere + to persist the state. A silent no-op would let a caller believe + ``resolve()`` succeeded and the thread is closed when, after + save/reopen, ``resolved`` is still ``False`` — a correctness lie. + A loud ``TypeError`` makes the format limitation explicit at the + call site instead of hiding data loss. + """ + if self.is_legacy: + raise TypeError( + "cannot resolve a legacy comment: the pre-2018 " + "PresentationML comment schema has no resolution attribute, " + "so the state cannot be persisted. Resolution is only " + "supported on modern threaded comments (issue #25 SF6)." + ) + self._cm.status = _RESOLVED_STATUS + + def reopen(self) -> None: + """Clear the resolved marker, reopening this threaded comment. + + The inverse of :meth:`resolve`. Raises the same |TypeError| on a + legacy-backed comment, for the same reason. + """ + if self.is_legacy: + raise TypeError( + "cannot reopen a legacy comment: legacy comments " + "have no resolution state (issue #25 SF6)." + ) + self._cm.status = None + + @property + def anchor_position(self) -> tuple[Length, Length] | None: + """The (x, y) slide-space anchor of this comment, or ``None``. + + For a modern comment created with an ``anchor`` shape, the shape's + id is stored on the ```` and this resolves it back to the + anchored shape's ``(left, top)`` as a |Length| pair. Legacy + comments carry an absolute ```` point instead. Returns + ``None`` for an unanchored comment or when the shape is gone. + """ + if self.is_legacy: + pos = self._cm.pos + if pos is None: + return None + return (Emu(int(pos.x)), Emu(int(pos.y))) + shape_id = self._cm.anchorShapeId + if shape_id is None: + return None + for shape in self._slide.shapes: + if shape.shape_id == shape_id: + left = shape.left if shape.left is not None else Emu(0) + top = shape.top if shape.top is not None else Emu(0) + return (Emu(int(left)), Emu(int(top))) + return None + + @property + def _anchor_shape_id(self) -> int | None: + """The anchored shape id for a modern comment, else ``None``. + + Internal helper for the per-shape filter (:attr:`BaseShape.comments`, + SF7). Legacy comments anchor to an absolute point, never a shape, so + they never participate in the per-shape filter. + """ + if self.is_legacy: + return None + return self._cm.anchorShapeId + + @property + def replies(self) -> "CommentReplies": + """The ordered reply thread under this comment. + + Always empty for a legacy comment — the pre-2018 schema has no + reply thread (SF4 documented legacy behavior). + """ + return CommentReplies(self._cm, self._slide) + + +class CommentReply: + """One reply in a threaded comment's thread.""" + + def __init__(self, reply: "CT_ThreadedCommentReply", slide: "Slide"): + self._reply = reply + self._slide = slide + + def __eq__(self, other: object) -> bool: + return isinstance(other, CommentReply) and other._reply is self._reply + + def __ne__(self, other: object) -> bool: + return not self.__eq__(other) + + @property + def text(self) -> str: + """Plain-text body of this reply.""" + return self._reply.text + + @property + def author(self) -> str | None: + """Display name of this reply's author, resolved authorId→name.""" + return self._slide._resolve_author_name(self._reply.authorId) + + @property + def created_at(self) -> datetime | None: + """Creation timestamp as a tz-aware ``datetime``, or ``None``.""" + return _parse_created(self._reply.created) + + +class CommentReplies: + """The ordered reply thread under a single threaded comment. + + A legacy ```` has no reply container, so against a legacy-backed + comment this collection is always empty and :meth:`add` raises — the + pre-2018 schema cannot persist a reply thread (issue #25 SF8, the SF4 + documented legacy behavior). + """ + + def __init__(self, cm: "CT_ThreadedComment | CT_Comment", slide: "Slide"): + self._cm = cm + self._slide = slide + + @property + def _is_legacy(self) -> bool: + from pptx.oxml.comments import CT_Comment as _CT_Comment + + return isinstance(self._cm, _CT_Comment) + + def __len__(self) -> int: + if self._is_legacy: + return 0 + replyLst = self._cm.replyLst + return 0 if replyLst is None else len(replyLst.reply_lst) + + def __iter__(self) -> Iterator[CommentReply]: + if self._is_legacy: + return + replyLst = self._cm.replyLst + if replyLst is None: + return + for reply in replyLst.reply_lst: + yield CommentReply(reply, self._slide) + + def __getitem__(self, idx: int) -> CommentReply: + return list(self)[idx] + + def add(self, text: str, author: str) -> CommentReply: + """Append a reply carrying `text` by `author`; return it. + + Threads under the parent comment, preserves reply order, and never + detaches the parent comment from its list (ISC-27 anti-criterion): + the parent ```` is mutated in place, never re-parented. + + :raises TypeError: on a legacy-backed comment — the pre-2018 schema + has no reply thread to append to (issue #25 SF8). + """ + if self._is_legacy: + raise TypeError( + "cannot reply to a legacy comment: the pre-2018 " + "schema has no reply thread. Replies are only supported on " + "modern threaded comments (issue #25 SF4/SF8)." + ) + author_guid = self._slide._get_or_add_author_guid(author) + reply = self._cm.add_reply(_new_guid(), author_guid, _now_iso()) + reply.text = text + return CommentReply(reply, self._slide) + + +class Comments: + """Per-slide collection of comments — legacy *and* modern. + + Returned by :attr:`Slide.comments`. Iterating yields |Comment| objects: + first any pre-2018 legacy ```` comments the source deck carried + (read-only proxies), then the modern ```` threaded comments, in + document order. Reading both families is what makes legacy↔modern + coexistence actually hold (issue #25 Wave 3, SF8 / ISC-44, ISC-67): + adding a modern comment never deletes or rewrites the legacy part, and a + save→reopen surfaces both. The backing |ModernCommentsPart| is created + and related to the slide lazily on the first :meth:`add`; the legacy + part is *never* created — python-pptx only ever writes modern comments. + """ + + def __init__(self, slide: "Slide"): + self._slide = slide + + def __len__(self) -> int: + return sum(1 for _ in self) + + def __iter__(self) -> Iterator[Comment]: + # ---legacy first (older model, read-only). Never creates the + # part — it only exists when the source deck already had one.--- + legacy_part = self._slide.part.legacy_comments_part_if_present + if legacy_part is not None: + for cm in legacy_part.iter_comments(): + yield Comment(cm, self._slide) + # ---then modern threaded comments--- + part = self._slide._modern_comments_part_if_present + if part is None: + return + for cm in part.iter_comments(): + yield Comment(cm, self._slide) + + def __getitem__(self, idx: int) -> Comment: + return list(self)[idx] + + def add(self, text: str, author: str, anchor=None) -> Comment: + """Add a threaded comment carrying `text` by `author`; return it. + + The first call lazily creates and relates the per-slide + |ModernCommentsPart| (mirrors ``SlidePart.notes_slide``). The + author is get-or-added on the presentation-level + |CommentAuthorsPart| by name with no duplicates (ISC-5). When + `anchor` is a shape, its id is recorded so + :attr:`Comment.anchor_position` can resolve it back later. + """ + part = self._slide.part.modern_comments_part + author_guid = self._slide._get_or_add_author_guid(author) + cm = part.add_comment(_new_guid(), author_guid, _now_iso(), text) + # GROUND TRUTH (2026-05-17): PowerPoint binds a modern comment to its + # slide via a `` carrying the slide's `/@id`. + # Without it the comment never renders in the Comments pane (issue + # #25 root cause). slide_id is the integer sldId from presentation.xml. + cm.set_slide_marker(self._slide.slide_id) + if anchor is not None and getattr(anchor, "shape_id", None) is not None: + cm.anchorShapeId = anchor.shape_id + return Comment(cm, self._slide) + + def remove(self, comment: Comment) -> None: + """Detach `comment` from this slide's threaded-comments part. + + Removing the last comment deliberately leaves a valid empty + ```` and keeps the slide↔part relationship intact + (ISC-21 anti-criterion) so the file stays openable. + """ + part = self._slide._modern_comments_part_if_present + if part is None: + return + part.remove_comment(comment._cm) diff --git a/src/pptx/opc/constants.py b/src/pptx/opc/constants.py index 19e0e6bf5..8738d3867 100644 --- a/src/pptx/opc/constants.py +++ b/src/pptx/opc/constants.py @@ -88,6 +88,19 @@ class CONTENT_TYPE: PML_TABLE_STYLES = ( "application/vnd.openxmlformats-officedocument.presentationml.tableStyles+xml" ) + # -- modern (2018) threaded-comments part. GROUND TRUTH (2026-05-17): + # -- captured from a comment PowerPoint for Mac itself authored+saved — + # -- the part is stamped `application/vnd.ms-powerpoint.comments+xml` + # -- (NOT `...threadedComments+xml`, which was an inference and is the + # -- string PowerPoint silently fails to recognize → empty Comments + # -- pane, issue #25). Symbol name kept for blast-radius; value fixed. + PML_THREADED_COMMENTS = "application/vnd.ms-powerpoint.comments+xml" + # -- modern (2018) threaded-comment AUTHORS part. Distinct from the legacy + # -- `commentAuthors.xml` (ECMA-376 §19.5, integer ids): the modern + # -- `/@authorId` is a GUID and must resolve to a + # -- `` in this part. Vendor MIME type from the [MS-PPTX] + # -- threaded-comments extension; partname is `/ppt/authors.xml`. + PML_AUTHORS = "application/vnd.ms-powerpoint.authors+xml" PML_TAGS = "application/vnd.openxmlformats-officedocument.presentationml.tags+xml" PML_TEMPLATE_MAIN = ( "application/vnd.openxmlformats-officedocument.presentationml.template.main+xml" @@ -301,6 +314,20 @@ class RELATIONSHIP_TYPE: SLIDE_UPDATE_INFO = ( "http://schemas.openxmlformats.org/officeDocument/2006/relationships/slideUpdateInfo" ) + # -- modern (2018) threaded-comments slide→part relationship. GROUND + # -- TRUTH (2026-05-17, PowerPoint-authored capture): the slide relates + # -- to its modern-comments part with `.../2018/10/relationships/comments` + # -- (NOT `.../threadedComment`). PowerPoint locates the comment part by + # -- THIS reltype off the slide; the prior `threadedComment` form is + # -- unrecognized → the part is never loaded → empty Comments pane. + THREADED_COMMENT = "http://schemas.microsoft.com/office/2018/10/relationships/comments" + # -- modern (2018) threaded-comment AUTHORS relationship. GROUND TRUTH + # -- (2026-05-17, PowerPoint-authored capture): the PRESENTATION part + # -- relates to `/ppt/authors.xml` with `.../2018/10/relationships/authors` + # -- (NOT `.../threadedCommentAuthors`). The bare `.../authors` form IS + # -- the one PowerPoint emits and resolves `/@authorId` GUIDs + # -- through — the earlier `threadedCommentAuthors` was an inference. + AUTHORS = "http://schemas.microsoft.com/office/2018/10/relationships/authors" STYLES = "http://schemas.openxmlformats.org/officeDocument/2006/relationships/styles" TABLE = "http://schemas.openxmlformats.org/officeDocument/2006/relationships/table" TABLE_SINGLE_CELLS = ( diff --git a/src/pptx/oxml/__init__.py b/src/pptx/oxml/__init__.py index 9d44a0b95..b9e70c6e0 100644 --- a/src/pptx/oxml/__init__.py +++ b/src/pptx/oxml/__init__.py @@ -212,6 +212,33 @@ def register_element_cls(nsptagname: str, cls: Type[BaseOxmlElement]): register_element_cls("c:xMode", CT_LayoutMode) +from pptx.oxml.comments import ( # noqa: E402 + CT_Author, + CT_AuthorList, + CT_Comment, + CT_CommentAuthor, + CT_CommentAuthorList, + CT_CommentList, + CT_CommentPosition, + CT_ThreadedComment, + CT_ThreadedCommentList, + CT_ThreadedCommentReply, + CT_ThreadedCommentReplyList, +) + +register_element_cls("p:cm", CT_Comment) +register_element_cls("p:cmAuthor", CT_CommentAuthor) +register_element_cls("p:cmAuthorLst", CT_CommentAuthorList) +register_element_cls("p:cmLst", CT_CommentList) +register_element_cls("p:pos", CT_CommentPosition) +register_element_cls("p188:author", CT_Author) +register_element_cls("p188:authorLst", CT_AuthorList) +register_element_cls("p188:cm", CT_ThreadedComment) +register_element_cls("p188:cmLst", CT_ThreadedCommentList) +register_element_cls("p188:reply", CT_ThreadedCommentReply) +register_element_cls("p188:replyLst", CT_ThreadedCommentReplyList) + + from pptx.oxml.coreprops import CT_CoreProperties # noqa: E402 register_element_cls("cp:coreProperties", CT_CoreProperties) diff --git a/src/pptx/oxml/comments.py b/src/pptx/oxml/comments.py new file mode 100644 index 000000000..84e6fd43a --- /dev/null +++ b/src/pptx/oxml/comments.py @@ -0,0 +1,550 @@ +"""Custom element classes for presentation comments (issue #25, Wave 1). + +Two comment families live here: + +* **Legacy comments** — the original PresentationML comment model + (ECMA-376 Part 1, §19.5: ``p:cmAuthorLst``/``p:cmAuthor`` in + ``commentAuthors.xml`` and ``p:cmLst``/``p:cm`` in a per-slide comments + part). One shared author list for the whole presentation; comments carry + an ``authorId`` foreign key into it. + +* **Modern threaded comments** — Microsoft's post-2018 model in the + ``http://schemas.microsoft.com/office/powerpoint/2018/8/main`` namespace + (prefix ``p188``). A ```` carries a GUID ``id``, an author GUID, + a ``created`` timestamp, a ```` and an optional + ```` of ```` children — this is the structure + that gives the feature its "threaded" name. Author identities for the + modern model live in a separate ``authors`` part (Wave 2). + +There is no upstream `scanny/python-pptx` prior art for any of this; the +element shapes below are derived directly from the OOXML / MS-OOXML +schemas and the registration conventions in ``pptx/oxml/slide.py``. +""" + +from __future__ import annotations + +import uuid +from typing import cast + +from pptx.oxml import parse_xml +from pptx.oxml.ns import nsdecls, qn +from pptx.oxml.simpletypes import XsdString, XsdUnsignedInt +from pptx.oxml.xmlchemy import ( + BaseOxmlElement, + OptionalAttribute, + RequiredAttribute, + ZeroOrMore, + ZeroOrOne, +) + +# DrawingML namespace used inside a ```` (shared by the modern +# threaded comment and its replies). The body is a minimal DrawingML +# text-body: a ```` followed by one ```` paragraph carrying a +# single ``/`` run for the plain-text payload. +_A_NS = "http://schemas.openxmlformats.org/drawingml/2006/main" + + +def _txBody_text(txBody) -> str: + """Return the concatenated plain text of every ``a:t`` under `txBody`.""" + if txBody is None: + return "" + return "".join(t.text or "" for t in txBody.iter(qn("a:t"))) + + +def _set_txBody_text(txBody, value: str) -> None: + """Replace `txBody`'s body with a single paragraph/run holding `value`. + + ```` is a DrawingML ``a:CT_TextBody`` whose schema REQUIRES + ```` as its first child. A txBody without it is silently + dropped by PowerPoint's threaded-comment reader — the comment never + appears in the Comments pane (no repair dialog, just absent). The + add-path builds the txBody via ``get_or_add_txBody()`` (a bare + ````), so this function must GUARANTEE the leading + ````, not merely preserve a pre-existing one. A trailing + ``value`` follows it. + """ + for p in list(txBody.findall(qn("a:p"))): + txBody.remove(p) + if txBody.find(qn("a:bodyPr")) is None: + txBody.insert(0, parse_xml('' % _A_NS)) + p = parse_xml( + '%s' % (_A_NS, _escape_xml_text(value)) + ) + txBody.append(p) + + +def _escape_xml_text(value: str) -> str: + """Minimal XML-text escaping for the comment-body fast path.""" + return value.replace("&", "&").replace("<", "<").replace(">", ">") + + +# -- legacy author list (commentAuthors.xml) ----------------------------------- + + +class CT_CommentAuthorList(BaseOxmlElement): + """`p:cmAuthorLst` element, root of the ``commentAuthors.xml`` part. + + ECMA-376 Part 1 §19.5.7. A presentation has at most one of these; every + legacy ``p:cm`` references an entry here by ``authorId``. + """ + + cmAuthor_lst: list[CT_CommentAuthor] + _add_cmAuthor: "callable" + + cmAuthor = ZeroOrMore("p:cmAuthor", successors=()) + + @classmethod + def new(cls) -> CT_CommentAuthorList: + """Return a new empty ```` element.""" + return cast(CT_CommentAuthorList, parse_xml("" % nsdecls("p"))) + + @property + def next_author_id(self) -> int: + """The lowest unused non-negative integer author id. + + ``p:cmAuthor/@id`` is an ``ST_Index`` (xsd:unsignedInt). PowerPoint's + own files start author ids at ``0`` and increment; allocating + ``max(existing) + 1`` (or ``0`` when empty) matches that and keeps + the ids stable across edits. + """ + used: set[int] = set() + for ca in self.cmAuthor_lst: + raw = ca.get("id") + if raw is not None and raw.lstrip("-").isdigit(): + used.add(int(raw)) + return (max(used) + 1) if used else 0 + + def add_author(self, name: str, initials: str = "") -> CT_CommentAuthor: + """Append and return a new ```` with the next free id.""" + new_id = self.next_author_id + author = cast(CT_CommentAuthor, self._add_cmAuthor()) + author.id = new_id + author.name = name + author.initials = initials + author.lastIdx = 0 + author.clrIdx = 0 + return author + + def get_or_add_author(self, name: str, initials: str = "") -> CT_CommentAuthor: + """Return the existing same-``name`` author, else add a new one. + + Name match is exact and case-sensitive — the same identity policy + PowerPoint uses for the legacy author list (ISC-5, no duplicates). + """ + for author in self.cmAuthor_lst: + if author.name == name: + return author + return self.add_author(name, initials) + + +class CT_CommentAuthor(BaseOxmlElement): + """`p:cmAuthor` element, one legacy comment author. ECMA-376 §19.5.1.""" + + id: int = RequiredAttribute( # pyright: ignore[reportAssignmentType] + "id", XsdUnsignedInt + ) + name: str = RequiredAttribute( # pyright: ignore[reportAssignmentType] + "name", XsdString + ) + initials: str = OptionalAttribute( # pyright: ignore[reportAssignmentType] + "initials", XsdString, default="" + ) + lastIdx: int = OptionalAttribute( # pyright: ignore[reportAssignmentType] + "lastIdx", XsdUnsignedInt, default=0 + ) + clrIdx: int = OptionalAttribute( # pyright: ignore[reportAssignmentType] + "clrIdx", XsdUnsignedInt, default=0 + ) + + +# -- modern author list (authors.xml, p188 / 2018/8/main) ---------------------- + + +def _author_guid_for_name(name: str) -> str: + """Deterministic, stable brace-wrapped uppercase GUID for `name`. + + A ``uuid5`` over the name gives a stable, round-trip-safe GUID so the + same author always maps to the same ``/@id`` (and therefore + the same ``/@authorId``) across edits. Resolution back to a + name reads the authors part rather than re-deriving, so correctness does + not depend on this derivation — it only keeps ids stable for dedup. + """ + return "{%s}" % str(uuid.uuid5(uuid.NAMESPACE_OID, "pptx-comment-author:" + name)).upper() + + +def _initials_for(name: str) -> str: + """Best-effort initials from a display `name` (e.g. "Ada Lovelace"→"AL").""" + parts = [p for p in name.split() if p] + if not parts: + return "" + return "".join(p[0] for p in parts).upper() + + +class CT_AuthorList(BaseOxmlElement): + """`p188:authorLst` element, root of the modern ``/ppt/authors.xml`` part. + + Microsoft `2018/8/main` schema ([MS-PPTX] threaded-comments extension). + Holds zero or more ```` entries keyed by GUID. Every modern + ````/```` references one of these by its GUID + ``authorId`` — distinct from the legacy ```` (integer + ids). A presentation has at most one of these parts. + """ + + author_lst: list[CT_Author] + _add_author: "callable" + + author = ZeroOrMore("p188:author", successors=()) + + @classmethod + def new(cls) -> CT_AuthorList: + """Return a new empty ```` element.""" + return cast(CT_AuthorList, parse_xml("" % nsdecls("p188"))) + + def get_or_add_author(self, name: str, initials: str = "") -> CT_Author: + """Return the existing same-``name`` author, else add a new one. + + Name match is exact and case-sensitive (ISC-5, no duplicates). The + new author's ``id`` is the deterministic GUID for `name`, so two + comments by the same author share one ```` and one GUID. + """ + for author in self.author_lst: + if author.name == name: + return author + new_author = cast(CT_Author, self._add_author()) + new_author.id = _author_guid_for_name(name) + new_author.name = name + new_author.initials = initials or _initials_for(name) + new_author.userId = name + new_author.providerId = "None" + return new_author + + +class CT_Author(BaseOxmlElement): + """`p188:author` element, one modern threaded-comment author. + + Microsoft `2018/8/main`. ``id`` is the GUID that ``/@authorId`` + foreign-keys; ``name`` is the display name; ``initials``, ``userId`` and + ``providerId`` carry the identity metadata PowerPoint writes + (``providerId`` is ``"None"`` for a locally-authored identity). + """ + + id: str = RequiredAttribute( # pyright: ignore[reportAssignmentType] + "id", XsdString + ) + name: str = RequiredAttribute( # pyright: ignore[reportAssignmentType] + "name", XsdString + ) + initials: str = OptionalAttribute( # pyright: ignore[reportAssignmentType] + "initials", XsdString, default="" + ) + userId: str = OptionalAttribute( # pyright: ignore[reportAssignmentType] + "userId", XsdString, default="" + ) + # providerId is RequiredAttribute (not Optional-with-default) so it is + # ALWAYS serialized. Real PowerPoint-authored `` elements + # always carry providerId; omitting it (which an Optional default would + # do) is NOT verified-equivalent on PowerPoint's read path for this MS + # extension schema and is a plausible repair trigger. + providerId: str = RequiredAttribute( # pyright: ignore[reportAssignmentType] + "providerId", XsdString + ) + + +# -- legacy comment list (per-slide comments part) ----------------------------- + + +class CT_CommentList(BaseOxmlElement): + """`p:cmLst` element, root of a legacy per-slide comments part. + + ECMA-376 Part 1 §19.5.8. + """ + + cm_lst: list[CT_Comment] + + cm = ZeroOrMore("p:cm", successors=()) + + @classmethod + def new(cls) -> CT_CommentList: + """Return a new empty ```` element.""" + return cast(CT_CommentList, parse_xml("" % nsdecls("p"))) + + +class CT_Comment(BaseOxmlElement): + """`p:cm` element, one legacy comment on a slide. ECMA-376 §19.5.6. + + ```` (anchor x/y) and ```` (the comment body) are the + two children; ``authorId`` foreign-keys the presentation-level author + list and ``idx`` is the per-author comment ordinal. + """ + + pos = ZeroOrOne("p:pos", successors=("p:text", "p:extLst")) + _text = ZeroOrOne("p:text", successors=("p:extLst",)) + + authorId: int = RequiredAttribute( # pyright: ignore[reportAssignmentType] + "authorId", XsdUnsignedInt + ) + dt: str = OptionalAttribute( # pyright: ignore[reportAssignmentType] + "dt", XsdString + ) + idx: int = RequiredAttribute( # pyright: ignore[reportAssignmentType] + "idx", XsdUnsignedInt + ) + + @property + def text(self) -> str: + """Plain-text body of this comment (```` content).""" + text_elm = self._text + if text_elm is None: + return "" + return text_elm.text or "" + + @text.setter + def text(self, value: str) -> None: + text_elm = self.get_or_add__text() + text_elm.text = value + + +class CT_CommentPosition(BaseOxmlElement): + """`p:pos` element — the slide-space anchor point of a legacy comment.""" + + x: int = RequiredAttribute("x", XsdUnsignedInt) # pyright: ignore[reportAssignmentType] + y: int = RequiredAttribute("y", XsdUnsignedInt) # pyright: ignore[reportAssignmentType] + + +# -- modern threaded comments (p188 / 2018/8/main) ----------------------------- + + +class CT_ThreadedComment(BaseOxmlElement): + """`p188:cm` element, a modern threaded comment. + + Microsoft `2018/8/main` schema. A top-level comment carries a GUID + ``id``, an author GUID ``authorId``, an ISO ``created`` timestamp, a + ```` rich-text body, and an optional ```` + holding the reply thread. ``status`` (e.g. ``"resolved"``) marks a + closed thread. + + GROUND TRUTH (2026-05-17, captured from a comment PowerPoint for Mac + authored+saved): the FIRST child of every top-level ```` is a + ```` slide-binding marker + (````, ns + ``http://schemas.microsoft.com/office/powerpoint/2013/main/command``). + This is how PowerPoint binds a comment to its slide — NOT the per-slide + relationship and NOT the fork-private ``extLst`` anchor. Omitting it + leaves the comment unbound and PowerPoint does not render it (issue #25 + empty-Comments-pane root cause). Replies do not carry their own marker. + """ + + # Child order is FIXED by [MS-PPTX] CT_Comment (authoritative spec, the + # contract PowerPoint implements): + # + # EG_CommentAnchor (pc:sldMkLst) minOccurs=1 + # pos (a:CT_Point2D) minOccurs=0 + # replyLst (CT_CommentReplyList) minOccurs=0 + # EG_CommentProperties (txBody, extLst) minOccurs=1 + # + # i.e. replyLst MUST precede txBody. Emitting AFTER + # (the pre-2026-05-17 order) is out-of-sequence: PowerPoint + # renders the parent comment but SILENTLY DROPS every reply. The successor + # tuples below encode this exact order (pos omitted — optional, unused). + sldMkLst = ZeroOrOne("pc:sldMkLst", successors=("p188:replyLst", "p188:txBody", "p188:extLst")) + replyLst: "CT_ThreadedCommentReplyList | None" = ZeroOrOne( # pyright: ignore + "p188:replyLst", successors=("p188:txBody", "p188:extLst") + ) + txBody = ZeroOrOne("p188:txBody", successors=("p188:extLst",)) + + id: str = RequiredAttribute( # pyright: ignore[reportAssignmentType] + "id", XsdString + ) + authorId: str = RequiredAttribute( # pyright: ignore[reportAssignmentType] + "authorId", XsdString + ) + created: str = RequiredAttribute( # pyright: ignore[reportAssignmentType] + "created", XsdString + ) + status: str = OptionalAttribute( # pyright: ignore[reportAssignmentType] + "status", XsdString + ) + extLst = ZeroOrOne("p188:extLst", successors=()) + + # ---Shape anchor (Wave-2 SF7). NOT a schema attribute: the 2018/8/main + # `` schema defines only id/authorId/created/status. An + # out-of-schema attribute on p188:cm is a plausible PowerPoint + # repair-dialog trigger (Cato audit), so the anchored shape id is + # stored in a `/` keyed by a fork-private URI + # — the OOXML-sanctioned extension mechanism that conformant + # consumers MUST ignore when the URI is unknown. + _ANCHOR_EXT_URI = "https://github.com/MHoroszowski/python-pptx/ns/comment-anchor" + _ANCHOR_NS = _ANCHOR_EXT_URI + _ANCHOR_TAG = "{%s}anchor" % _ANCHOR_NS + + @property + def anchorShapeId(self) -> int | None: + """Anchored shape id, read from the fork extLst extension, else None.""" + extLst = self.extLst + if extLst is None: + return None + for ext in extLst.findall(qn("p188:ext")): + if ext.get("uri") != self._ANCHOR_EXT_URI: + continue + anchor = ext.find(self._ANCHOR_TAG) + if anchor is None: + return None + raw = anchor.get("shapeId") + return int(raw) if raw is not None and raw.lstrip("-").isdigit() else None + return None + + @anchorShapeId.setter + def anchorShapeId(self, shape_id: int) -> None: + extLst = self.get_or_add_extLst() + for ext in extLst.findall(qn("p188:ext")): + if ext.get("uri") == self._ANCHOR_EXT_URI: + anchor = ext.find(self._ANCHOR_TAG) + if anchor is None: + anchor = ext.makeelement(self._ANCHOR_TAG, {}) + ext.append(anchor) + anchor.set("shapeId", str(shape_id)) + return + ext = extLst.makeelement(qn("p188:ext"), {"uri": self._ANCHOR_EXT_URI}) + anchor = ext.makeelement(self._ANCHOR_TAG, {"shapeId": str(shape_id)}) + ext.append(anchor) + extLst.append(ext) + + def set_slide_marker(self, slide_id: int, c_id: int = 0) -> None: + """Set the ```` binding this comment to slide `slide_id`. + + Builds `` + `` and places it as the FIRST child of this + ```` (before ````), matching the structure + PowerPoint itself emits. ``slide_id`` is the slide's + ``/@id`` (e.g. 256 for the first slide); ``c_id`` is the + collaboration/change id PowerPoint stamps ``0`` for a locally + authored comment. Replaces any pre-existing marker (idempotent). + """ + existing = self.sldMkLst + if existing is not None: + self.remove(existing) + self.insert( + 0, + parse_xml( + "' % (nsdecls("pc"), c_id, slide_id) + ), + ) + + @classmethod + def new(cls, comment_id: str, author_id: str, created: str) -> CT_ThreadedComment: + """Return a minimal-valid ```` with an empty text body.""" + xml = ( + '' + "" + '' + '' + "" + "" % (nsdecls("p188"), comment_id, author_id, created) + ) + return cast(CT_ThreadedComment, parse_xml(xml)) + + @property + def text(self) -> str: + """Plain-text body of this threaded comment.""" + return _txBody_text(self.txBody) + + @text.setter + def text(self, value: str) -> None: + self.get_or_add_txBody() + _set_txBody_text(self.txBody, value) + + def add_reply(self, reply_id: str, author_id: str, created: str) -> CT_ThreadedCommentReply: + """Append and return a new ```` in this comment's thread. + + The ```` container is created on first reply + (``ZeroOrOne``). Replies are flat children of the parent comment — + the modern schema has no reply-to-reply nesting. + """ + replyLst = self.get_or_add_replyLst() + reply = cast(CT_ThreadedCommentReply, replyLst._add_reply()) + reply.id = reply_id + reply.authorId = author_id + reply.created = created + reply.get_or_add_txBody() + return reply + + +class CT_ThreadedCommentReplyList(BaseOxmlElement): + """`p188:replyLst` element — ordered replies under a threaded comment.""" + + reply_lst: list[CT_ThreadedCommentReply] + + reply = ZeroOrMore("p188:reply", successors=()) + + +class CT_ThreadedCommentReply(BaseOxmlElement): + """`p188:reply` element — one reply in a comment thread. + + Same attribute/body shape as the parent ```` minus the nested + ``replyLst`` (replies are flat under their parent comment). + """ + + txBody = ZeroOrOne("p188:txBody", successors=("p188:extLst",)) + + id: str = RequiredAttribute( # pyright: ignore[reportAssignmentType] + "id", XsdString + ) + authorId: str = RequiredAttribute( # pyright: ignore[reportAssignmentType] + "authorId", XsdString + ) + created: str = RequiredAttribute( # pyright: ignore[reportAssignmentType] + "created", XsdString + ) + status: str = OptionalAttribute( # pyright: ignore[reportAssignmentType] + "status", XsdString + ) + + @property + def text(self) -> str: + """Plain-text body of this reply.""" + return _txBody_text(self.txBody) + + @text.setter + def text(self, value: str) -> None: + self.get_or_add_txBody() + _set_txBody_text(self.txBody, value) + + +class CT_ThreadedCommentList(BaseOxmlElement): + """`p188:cmLst` element — root of a modern threaded-comments part. + + Microsoft `2018/8/main` schema. Holds zero or more ```` + threaded comments. Registering this class (rather than letting the part + round-trip as a plain element) is what gives the reopened + |ModernCommentsPart| a working ``cm_lst`` accessor — without it a + save→reopen cycle loses the comment-list API (issue #25 Wave 2). + """ + + cm_lst: list[CT_ThreadedComment] + _add_cm: "callable" + + cm = ZeroOrMore("p188:cm", successors=()) + + @classmethod + def new(cls) -> CT_ThreadedCommentList: + """Return a new empty ```` element.""" + return cast(CT_ThreadedCommentList, parse_xml("" % nsdecls("p188"))) + + def add_comment(self, comment_id: str, author_id: str, created: str) -> CT_ThreadedComment: + """Append and return a new ```` with an empty text body.""" + cm = cast(CT_ThreadedComment, self._add_cm()) + cm.id = comment_id + cm.authorId = author_id + cm.created = created + cm.get_or_add_txBody() + return cm + + def remove_comment(self, cm: CT_ThreadedComment) -> None: + """Detach `cm` from this list, leaving a valid (possibly empty) list. + + Removing the last comment deliberately does NOT drop the part or its + relationship — an empty ```` is valid OOXML and keeps + the slide rels intact (issue #25 Wave 2, ISC-21 anti-criterion). + """ + self.remove(cm) diff --git a/src/pptx/oxml/ns.py b/src/pptx/oxml/ns.py index 4b0ff62da..de33a8480 100644 --- a/src/pptx/oxml/ns.py +++ b/src/pptx/oxml/ns.py @@ -22,6 +22,8 @@ "op": "http://schemas.openxmlformats.org/officeDocument/2006/custom-properties", "p": "http://schemas.openxmlformats.org/presentationml/2006/main", "p14": "http://schemas.microsoft.com/office/powerpoint/2010/main", + "p188": "http://schemas.microsoft.com/office/powerpoint/2018/8/main", + "pc": "http://schemas.microsoft.com/office/powerpoint/2013/main/command", "pd": "http://schemas.openxmlformats.org/drawingml/2006/presentationDrawing", "pic": "http://schemas.openxmlformats.org/drawingml/2006/picture", "pr": "http://schemas.openxmlformats.org/package/2006/relationships", diff --git a/src/pptx/parts/comments.py b/src/pptx/parts/comments.py new file mode 100644 index 000000000..d25c317ae --- /dev/null +++ b/src/pptx/parts/comments.py @@ -0,0 +1,203 @@ +"""Part classes for presentation comments (issue #25). + +Four parts spanning the two comment families in ``pptx/oxml/comments.py``. +The modern family needs a *separate* GUID-keyed authors part +(:class:`AuthorsPart`, ``/ppt/authors.xml``) — reusing the legacy +integer-keyed ``commentAuthors.xml`` for modern ```` author refs +left the GUID ``authorId`` dangling and triggered the PowerPoint repair +dialog. The legacy :class:`CommentAuthorsPart` stays for legacy ```` +comments only. + +* :class:`CommentAuthorsPart` — the singleton ``/ppt/commentAuthors.xml`` + holding the legacy ````. Content-type + ``CT.PML_COMMENT_AUTHORS``; related from the presentation part via + ``RT.COMMENT_AUTHORS``. + +* :class:`CommentsPart` — a per-slide legacy comments part wrapping + ```` (content-type ``CT.PML_COMMENTS``). PowerPoint names these + ``/ppt/comments/commentN.xml`` (older builds used + ``/ppt/comments/comment1.xml`` directly under the slide rel). + +* :class:`ModernCommentsPart` — the post-2018 threaded comments part. + + Partname choice: PowerPoint emits ``/ppt/comments/modernComment_.xml`` + (one threaded-comments part per slide, the slide stem embedded in the + filename). `python-pptx` cannot know the human slide name at part-creation + time, so the public ``new()`` takes an explicit ``PackURI`` and Wave-2 + wiring will allocate ``/ppt/comments/modernComment_slideN.xml`` keyed off + the owning slide's partname index. Content-type + ``CT.PML_THREADED_COMMENTS`` (``application/vnd.ms-powerpoint.threadedComments+xml``) + and relationship ``RT.THREADED_COMMENT`` are the vendor identifiers + PowerPoint stamps — there is no ECMA-376 equivalent because threaded + comments are a Microsoft extension, not part of the base standard. + +No upstream `scanny/python-pptx` prior art; built from spec following the +``pptx/parts/coreprops.py`` / ``pptx/parts/slide.py`` part conventions. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Iterator + +from pptx.opc.constants import CONTENT_TYPE as CT +from pptx.opc.package import XmlPart +from pptx.opc.packuri import PackURI +from pptx.oxml.comments import ( + CT_Author, + CT_AuthorList, + CT_Comment, + CT_CommentAuthor, + CT_CommentAuthorList, + CT_CommentList, + CT_ThreadedComment, + CT_ThreadedCommentList, +) + +if TYPE_CHECKING: + from pptx.package import Package + + +class CommentAuthorsPart(XmlPart): + """The presentation-wide legacy comment-authors part. + + Corresponds to ``/ppt/commentAuthors.xml``. There is at most one per + package; it is related from the presentation part with + ``RT.COMMENT_AUTHORS``. + """ + + _element: CT_CommentAuthorList + + @classmethod + def new(cls, package: "Package") -> "CommentAuthorsPart": + """Return a new empty |CommentAuthorsPart| for `package`.""" + return cls( + PackURI("/ppt/commentAuthors.xml"), + CT.PML_COMMENT_AUTHORS, + package, + CT_CommentAuthorList.new(), + ) + + def add_author(self, name: str, initials: str = "") -> CT_CommentAuthor: + """Add a ```` for `name`, allocating the next free id.""" + return self._element.add_author(name, initials) + + def get_or_add_author(self, name: str, initials: str = "") -> CT_CommentAuthor: + """Return the existing same-`name` author, else add a new one (ISC-5).""" + return self._element.get_or_add_author(name, initials) + + def iter_authors(self) -> Iterator[CT_CommentAuthor]: + """Generate each ```` element in document order.""" + yield from self._element.cmAuthor_lst + + +class AuthorsPart(XmlPart): + """The presentation-wide MODERN threaded-comment authors part. + + Corresponds to ``/ppt/authors.xml`` (content-type ``CT.PML_AUTHORS``, + ``application/vnd.ms-powerpoint.authors+xml``). There is at most one per + package; it is related from the **presentation** part with ``RT.AUTHORS`` + (``…/office/2018/10/relationships/authors``). + + This is the part the modern ``/@authorId`` GUID resolves into. + It is *distinct* from the legacy |CommentAuthorsPart| + (``/ppt/commentAuthors.xml``, integer ids) — that part stays for legacy + ```` comments only. A modern-only deck must carry *this* part and + must NOT carry an orphaned ``commentAuthors.xml`` (issue #25 repair-dialog + root cause: a ```` GUID with no ```` to resolve to). + + No upstream `scanny/python-pptx` prior art; built from the [MS-PPTX] + threaded-comments extension following this fork's part conventions. + """ + + _element: CT_AuthorList + + @classmethod + def new(cls, package: "Package") -> "AuthorsPart": + """Return a new empty |AuthorsPart| for `package`.""" + return cls( + PackURI("/ppt/authors.xml"), + CT.PML_AUTHORS, + package, + CT_AuthorList.new(), + ) + + def get_or_add_author(self, name: str, initials: str = "") -> CT_Author: + """Return the existing same-`name` author, else add a new one (ISC-5). + + The returned author's ``id`` is the deterministic GUID the modern + ````/```` ``authorId`` must equal. + """ + return self._element.get_or_add_author(name, initials) + + def iter_authors(self) -> Iterator[CT_Author]: + """Generate each ```` element in document order.""" + yield from self._element.author_lst + + +class CommentsPart(XmlPart): + """A legacy per-slide comments part wrapping ````. + + Content-type ``CT.PML_COMMENTS``; one per slide that has legacy + comments, related from the slide via ``RT.COMMENTS``. + """ + + _element: CT_CommentList + + @classmethod + def new(cls, package: "Package", partname: PackURI) -> "CommentsPart": + """Return a new empty |CommentsPart| at `partname` for `package`.""" + return cls(partname, CT.PML_COMMENTS, package, CT_CommentList.new()) + + def iter_comments(self) -> Iterator[CT_Comment]: + """Generate each legacy ```` element in document order. + + Read-only — python-pptx never writes legacy comments; this exists so + a deck authored elsewhere keeps its legacy feedback visible through + ``slide.comments`` (issue #25 Wave 3, SF8 / ISC-44, ISC-67). + """ + yield from self._element.cm_lst + + +class ModernCommentsPart(XmlPart): + """A modern (2018) threaded-comments part. + + Root element is ```` in the + ``http://schemas.microsoft.com/office/powerpoint/2018/8/main`` + namespace, holding zero or more ```` threaded comments. + Content-type ``CT.PML_THREADED_COMMENTS``; related from the slide via + ``RT.THREADED_COMMENT``. + """ + + _element: CT_ThreadedCommentList + + @classmethod + def new(cls, package: "Package", partname: PackURI) -> "ModernCommentsPart": + """Return a new empty |ModernCommentsPart| at `partname`. + + The body is an empty ```` (the modern container element, + a registered |CT_ThreadedCommentList| so the part keeps a working + ``cm_lst`` accessor across a save→reopen cycle) — Wave 2 appends + ```` children when a slide gains threaded comments. + """ + return cls( + partname, + CT.PML_THREADED_COMMENTS, + package, + CT_ThreadedCommentList.new(), + ) + + def add_comment( + self, comment_id: str, author_id: str, created: str, text: str + ) -> CT_ThreadedComment: + """Append a ```` carrying `text` and return its element.""" + cm = self._element.add_comment(comment_id, author_id, created) + cm.text = text + return cm + + def remove_comment(self, cm: CT_ThreadedComment) -> None: + """Detach `cm`; an empty ```` remains valid (ISC-21).""" + self._element.remove_comment(cm) + + def iter_comments(self) -> Iterator[CT_ThreadedComment]: + """Generate each ```` element in document order.""" + yield from self._element.cm_lst diff --git a/src/pptx/parts/presentation.py b/src/pptx/parts/presentation.py index f3c8bc2eb..b87fa0488 100644 --- a/src/pptx/parts/presentation.py +++ b/src/pptx/parts/presentation.py @@ -2,7 +2,7 @@ from __future__ import annotations -from typing import IO, TYPE_CHECKING, Iterable +from typing import IO, TYPE_CHECKING, Iterable, cast from pptx.opc.constants import RELATIONSHIP_TYPE as RT from pptx.opc.package import XmlPart @@ -14,6 +14,7 @@ if TYPE_CHECKING: from pptx.custom_properties import CustomProperties from pptx.custom_xml import CustomXmlParts + from pptx.parts.comments import AuthorsPart, CommentAuthorsPart from pptx.parts.coreprops import CorePropertiesPart from pptx.slide import HandoutMaster, NotesMaster, Slide, SlideLayout, SlideMaster @@ -125,6 +126,73 @@ def handout_master_part(self) -> HandoutMasterPart: "handout master template ships in this fork yet" ) from e + @property + def has_comment_authors(self) -> bool: + """`True` if this presentation has a legacy comment-authors part. + + Non-mutating — unlike :attr:`comment_authors_part`, this never + creates the part. Mirrors the non-mutating ``has_*`` accessors. + """ + try: + self.part_related_by(RT.COMMENT_AUTHORS) + except KeyError: + return False + return True + + @lazyproperty + def comment_authors_part(self) -> CommentAuthorsPart: + """The presentation-wide |CommentAuthorsPart|, lazily created. + + Mirrors :attr:`notes_master_part`: try the existing + ``RT.COMMENT_AUTHORS`` relationship and, only on |KeyError|, create + the singleton ``/ppt/commentAuthors.xml`` part and relate the + presentation to it. The same single part is returned on every call; + modern threaded comments reuse this one author list (one identity + registry per presentation, ISC-5 no-dup policy). + """ + from pptx.parts.comments import CommentAuthorsPart + + try: + return cast("CommentAuthorsPart", self.part_related_by(RT.COMMENT_AUTHORS)) + except KeyError: + comment_authors_part = CommentAuthorsPart.new(self.package) + self.relate_to(comment_authors_part, RT.COMMENT_AUTHORS) + return comment_authors_part + + @property + def has_authors(self) -> bool: + """`True` if this presentation has a MODERN authors part. + + Non-mutating — unlike :attr:`authors_part`, this never creates the + part. Mirrors :attr:`has_comment_authors` for the modern + ``/ppt/authors.xml`` (issue #25). + """ + try: + self.part_related_by(RT.AUTHORS) + except KeyError: + return False + return True + + @lazyproperty + def authors_part(self) -> AuthorsPart: + """The presentation-wide MODERN |AuthorsPart|, lazily created. + + Mirrors :attr:`comment_authors_part` but for the modern + ``/ppt/authors.xml`` (``RT.AUTHORS``). This is the GUID-keyed author + list every modern ``/@authorId`` resolves into. Distinct + from the legacy integer-keyed ``commentAuthors.xml`` — modern + threaded comments must NOT use the legacy part (issue #25 + repair-dialog root cause: orphaned modern author GUIDs). + """ + from pptx.parts.comments import AuthorsPart + + try: + return cast("AuthorsPart", self.part_related_by(RT.AUTHORS)) + except KeyError: + authors_part = AuthorsPart.new(self.package) + self.relate_to(authors_part, RT.AUTHORS) + return authors_part + @lazyproperty def presentation(self): """ diff --git a/src/pptx/parts/slide.py b/src/pptx/parts/slide.py index e1cbc602e..fd101cbe6 100644 --- a/src/pptx/parts/slide.py +++ b/src/pptx/parts/slide.py @@ -17,6 +17,7 @@ from pptx.oxml.slide import CT_NotesMaster, CT_NotesSlide, CT_Slide, CT_SlideLayout from pptx.oxml.theme import CT_OfficeStyleSheet from pptx.parts.chart import ChartPart +from pptx.parts.comments import CommentsPart, ModernCommentsPart from pptx.parts.embeddedpackage import EmbeddedPackagePart from pptx.parts.image import Image, ImagePart from pptx.slide import HandoutMaster, NotesMaster, NotesSlide, Slide, SlideLayout, SlideMaster @@ -301,6 +302,83 @@ def _add_notes_slide_part(self): self.relate_to(notes_slide_part, RT.NOTES_SLIDE) return notes_slide_part + @property + def has_modern_comments(self) -> bool: + """`True` if this slide has a modern (2018) threaded-comments part. + + Non-mutating — unlike :attr:`modern_comments_part`, this never + creates the part. Mirrors :attr:`has_notes_slide`. + """ + try: + self.part_related_by(RT.THREADED_COMMENT) + except KeyError: + return False + return True + + @lazyproperty + def modern_comments_part(self) -> ModernCommentsPart: + """The |ModernCommentsPart| for this slide, lazily created. + + Mirrors :attr:`notes_slide` exactly: try the existing + ``RT.THREADED_COMMENT`` relationship and, only on |KeyError|, create + the part and relate the slide to it. The per-slide partname is + ``/ppt/comments/modernComment_slideN.xml`` where ``N`` is THIS + slide's own partname index (e.g. ``slide3.xml`` → + ``modernComment_slide3.xml``) — matching PowerPoint's own naming and + keeping one threaded-comments part per slide. The same single part + is returned on every call. + """ + try: + return cast("ModernCommentsPart", self.part_related_by(RT.THREADED_COMMENT)) + except KeyError: + return self._add_modern_comments_part() + + def _add_modern_comments_part(self) -> ModernCommentsPart: + """Create + relate a fresh |ModernCommentsPart| for this slide. + + Caller guarantees no ``RT.THREADED_COMMENT`` rel exists yet. The + partname index is keyed off this slide's own ``slideN.xml`` index so + the comments part travels with its slide; ``next_partname`` keeps it + collision-safe if that exact name is somehow already taken. + """ + slide_idx = self.partname.idx or 1 + candidate = PackURI("/ppt/comments/modernComment_slide%d.xml" % slide_idx) + existing = {p.partname for p in self.package.iter_parts()} + if candidate in existing: + candidate = self.package.next_partname("/ppt/comments/modernComment_slide%d.xml") + modern_comments_part = ModernCommentsPart.new(self.package, candidate) + self.relate_to(modern_comments_part, RT.THREADED_COMMENT) + return modern_comments_part + + @property + def has_legacy_comments(self) -> bool: + """`True` if this slide has a legacy (pre-2018) ```` part. + + Non-mutating — never creates the part. Mirrors + :attr:`has_modern_comments`. python-pptx only *writes* modern + threaded comments, but a deck authored elsewhere can carry a legacy + per-slide comments part; the read path (issue #25 Wave 3, SF8 / + ISC-44, ISC-67) must surface those rather than silently drop them. + """ + try: + self.part_related_by(RT.COMMENTS) + except KeyError: + return False + return True + + @property + def legacy_comments_part_if_present(self) -> "CommentsPart | None": + """The slide's legacy |CommentsPart| if related, else ``None``. + + Strictly non-mutating read accessor for legacy↔modern coexistence + (SF8). The legacy part is never created by python-pptx — it only + exists when the source deck already had one. + """ + try: + return cast("CommentsPart", self.part_related_by(RT.COMMENTS)) + except KeyError: + return None + def duplicate(self) -> SlidePart: """Return a new |SlidePart| that is a deep copy of this one. diff --git a/src/pptx/shapes/base.py b/src/pptx/shapes/base.py index bcf687290..356ed7b5f 100644 --- a/src/pptx/shapes/base.py +++ b/src/pptx/shapes/base.py @@ -18,6 +18,9 @@ _LXML_XPATH = _Element.xpath if TYPE_CHECKING: + from collections.abc import Iterator + + from pptx.comments import Comment, Comments from pptx.enum.shapes import MSO_SHAPE_TYPE, PP_PLACEHOLDER from pptx.oxml.shapes import ShapeElement from pptx.oxml.shapes.shared import CT_Placeholder @@ -280,6 +283,23 @@ def shape_id(self) -> int: """ return self._element.shape_id + @property + def comments(self) -> "_ShapeComments": + """The comments anchored to *this* shape (issue #25 Wave 3, SF7). + + A filtered, read-only view over the owning slide's + ``slide.comments`` that yields only the modern threaded comments + whose anchor shape id matches this shape's :attr:`shape_id`. + Iterable and ``len()``-able; an empty (not error) collection when + the shape has no comments (ISC-39). A comment anchored to another + shape never appears here (ISC-40) — anchoring is by stable shape + id, so the filter survives a save→reopen. Legacy ```` + comments anchor to an absolute point rather than a shape and so + never participate in a per-shape filter. + """ + slide = self.part.slide # pyright: ignore[reportAttributeAccessIssue] + return _ShapeComments(slide.comments, self.shape_id) + @property def shape_type(self) -> MSO_SHAPE_TYPE: """A member of MSO_SHAPE_TYPE classifying this shape by type. @@ -340,3 +360,29 @@ def type(self) -> PP_PLACEHOLDER: A member of the :ref:`PpPlaceholderType` enumeration, e.g. PP_PLACEHOLDER.CHART """ return self._ph.type + + +class _ShapeComments: + """A read-only, per-shape filtered view over a slide's comments. + + Backs :attr:`BaseShape.comments` (issue #25 Wave 3, SF7). Wraps the + slide's |Comments| collection and yields only the comments whose anchor + resolves to one specific shape id. Iterable and ``len()``-able; indexing + is supported for convenience. Never mutates the package — anchoring is + established at ``slide.comments.add(..., anchor=shape)`` time. + """ + + def __init__(self, comments: "Comments", shape_id: int): + self._comments = comments + self._shape_id = shape_id + + def __iter__(self) -> "Iterator[Comment]": + for comment in self._comments: + if comment._anchor_shape_id == self._shape_id: # noqa: SLF001 + yield comment + + def __len__(self) -> int: + return sum(1 for _ in self) + + def __getitem__(self, idx: int) -> "Comment": + return list(self)[idx] diff --git a/src/pptx/slide.py b/src/pptx/slide.py index b560b29cb..7e7eb9eae 100644 --- a/src/pptx/slide.py +++ b/src/pptx/slide.py @@ -23,6 +23,7 @@ from pptx.util import Inches, Length, Pt, lazyproperty if TYPE_CHECKING: + from pptx.comments import Comments from pptx.oxml.presentation import CT_SlideIdList, CT_SlideMasterIdList from pptx.oxml.slide import ( CT_CommonSlideData, @@ -396,6 +397,78 @@ def footer(self, value: str | None) -> None: placeholder.text_frame.text = value + @lazyproperty + def comments(self) -> Comments: + """The |Comments| collection of modern threaded comments on this slide. + + Iterable in document order, ``len()``-able, with + ``add(text, author, anchor=None)`` and ``remove(comment)``. The + backing modern-comments part is created lazily on the first + ``add`` — reading an empty collection never mutates the package. + """ + from pptx.comments import Comments + + return Comments(self) + + @property + def _modern_comments_part_if_present(self): + """The slide's |ModernCommentsPart| if related, else ``None``. + + Non-mutating helper for the |Comments| collection — read paths + (``__len__``/``__iter__``/``remove``) must never create the part. + """ + if not self.part.has_modern_comments: + return None + return self.part.modern_comments_part + + def _get_or_add_author_guid(self, name: str) -> str: + """Get-or-add `name` on the MODERN authors part; return its GUID. + + The modern ````/```` ``authorId`` is a GUID; it + MUST resolve to a ```` in the presentation-level modern + ``/ppt/authors.xml`` part. Writing the author there (not the legacy + integer-keyed ``commentAuthors.xml``) is the issue-#25 repair-dialog + fix: previously a uuid5 GUID was stamped on the comment while the + author was only written to the legacy part, leaving the GUID + dangling. Reuses an existing same-name author with no duplicate + (ISC-5) and returns that author's stored GUID. + """ + authors_part = self.part.package.presentation_part.authors_part + return authors_part.get_or_add_author(name).id + + def _resolve_author_name(self, author_guid: str) -> str | None: + """Resolve a modern comment ``authorId`` GUID back to a display name. + + Reads the modern ``/ppt/authors.xml`` part (the GUID-keyed author + list) — the authors part is the source of truth across a + save→reopen, not a re-derivation. Returns ``None`` when no + ```` carries `author_guid` (externally-edited or + malformed file). + """ + pres_part = self.part.package.presentation_part + if not pres_part.has_authors: + return None + for author in pres_part.authors_part.iter_authors(): + if author.id == author_guid: + return author.name + return None + + def _resolve_legacy_author_name(self, author_id: int) -> str | None: + """Resolve a *legacy* ```` integer ``authorId`` to a name. + + Legacy comments key the author by the integer ``id`` on the + presentation ```` (not the modern deterministic + GUID). Used only by legacy-backed |Comment| proxies for SF8 + coexistence. Returns ``None`` when no author has that id. + """ + pres_part = self.part.package.presentation_part + if not pres_part.has_comment_authors: + return None + for author in pres_part.comment_authors_part.iter_authors(): + if author.id == author_id: + return author.name + return None + @lazyproperty def placeholders(self) -> SlidePlaceholders: """Sequence of placeholder shapes in this slide.""" diff --git a/tests/test_issue25_comments.py b/tests/test_issue25_comments.py new file mode 100644 index 000000000..e78f6362d --- /dev/null +++ b/tests/test_issue25_comments.py @@ -0,0 +1,806 @@ +"""Wave-1 tests for issue #25 (Threaded Comments & Review). + +Covers the oxml element classes and part classes that form the foundation +(SF1 + SF2, ISA ISC-1..12). `Slide.comments` is deliberately NOT exercised +here — that is Wave 2. + +TDD ordering per repo CLAUDE.md §3: these tests are written to fail first. +""" + +from __future__ import annotations + +import io + +import pytest + +from pptx import Presentation +from pptx.opc.constants import CONTENT_TYPE as CT +from pptx.opc.constants import RELATIONSHIP_TYPE as RT +from pptx.opc.package import PartFactory +from pptx.opc.packuri import PackURI +from pptx.oxml import parse_xml +from pptx.oxml.comments import ( + CT_Comment, + CT_CommentAuthor, + CT_CommentAuthorList, + CT_CommentList, + CT_ThreadedComment, +) +from pptx.oxml.ns import nsdecls, nsuri, qn +from pptx.parts.comments import ( + CommentAuthorsPart, + CommentsPart, + ModernCommentsPart, +) + +# -- oxml: legacy author list -------------------------------------------------- + + +class DescribeCT_CommentAuthorList: + def it_parses_a_hand_built_cmAuthorLst(self): + xml = ( + "" + '' + "" % nsdecls("p") + ) + elm = parse_xml(xml) + assert isinstance(elm, CT_CommentAuthorList) + authors = elm.cmAuthor_lst + assert len(authors) == 1 + assert isinstance(authors[0], CT_CommentAuthor) + assert authors[0].id == 0 + assert authors[0].name == "Ada" + assert authors[0].initials == "AL" + + +# -- oxml: legacy comment list ------------------------------------------------- + + +class DescribeCT_CommentList: + def it_parses_a_hand_built_cmLst(self): + xml = ( + "" + '' + '' + "Hello legacy" + "" + "" % nsdecls("p") + ) + elm = parse_xml(xml) + assert isinstance(elm, CT_CommentList) + comments = elm.cm_lst + assert len(comments) == 1 + cm = comments[0] + assert isinstance(cm, CT_Comment) + assert cm.authorId == 0 + assert cm.idx == 1 + assert cm.text == "Hello legacy" + + +# -- oxml: modern threaded comment --------------------------------------------- + + +class DescribeModernThreadedNamespace: + def it_registers_the_2018_8_main_namespace(self): + assert nsuri("p188") == "http://schemas.microsoft.com/office/powerpoint/2018/8/main" + # qn() must resolve a p188-prefixed tag to its Clark name + assert qn("p188:cm").startswith( + "{http://schemas.microsoft.com/office/powerpoint/2018/8/main}" + ) + + +class DescribeCT_ThreadedComment: + def it_parses_a_hand_built_p188_cm(self): + xml = ( + '' + "" + '' + '' + "modern comment" + "" + "" % nsdecls("p188") + ) + elm = parse_xml(xml) + assert isinstance(elm, CT_ThreadedComment) + assert elm.id == "{2E5D6F1A-0000-0000-0000-000000000001}" + assert elm.authorId == "{11111111-1111-1111-1111-111111111111}" + assert elm.created == "2026-05-16T12:00:00.000" + + def it_parses_a_p188_cm_with_reply(self): + xml = ( + '' + "" + '' + "" + "" + '' + "" + '' + "" + "" + "" + "" % nsdecls("p188") + ) + elm = parse_xml(xml) + assert isinstance(elm, CT_ThreadedComment) + assert elm.replyLst is not None + assert len(elm.replyLst.reply_lst) == 1 + + +# -- parts: comment authors ---------------------------------------------------- + + +class DescribeCommentAuthorsPart: + def it_creates_an_empty_part_via_new(self): + prs = Presentation() + part = CommentAuthorsPart.new(prs.part.package) + assert part.content_type == CT.PML_COMMENT_AUTHORS + assert str(part.partname) == "/ppt/commentAuthors.xml" + assert list(part.iter_authors()) == [] + + def it_allocates_the_next_free_id_on_add_author(self): + prs = Presentation() + part = CommentAuthorsPart.new(prs.part.package) + a0 = part.add_author("Ada", "AL") + a1 = part.add_author("Babbage", "CB") + assert a0.id == 0 + assert a1.id == 1 + assert a0.name == "Ada" + assert [a.name for a in part.iter_authors()] == ["Ada", "Babbage"] + + def it_dedupes_same_name_on_get_or_add_author(self): + # ISC-5: get_or_add_author reuses an existing same-name author id. + prs = Presentation() + part = CommentAuthorsPart.new(prs.part.package) + first = part.get_or_add_author("Ada") + again = part.get_or_add_author("Ada") + assert first.id == again.id + assert len(list(part.iter_authors())) == 1 + + def it_round_trips_through_a_presentation_save_reopen(self): + prs = Presentation() + pres_part = prs.part + authors_part = CommentAuthorsPart.new(pres_part.package) + authors_part.add_author("Ada", "AL") + pres_part.relate_to(authors_part, RT.COMMENT_AUTHORS) + + stream = io.BytesIO() + prs.save(stream) + stream.seek(0) + + prs2 = Presentation(stream) + reloaded = prs2.part.part_related_by(RT.COMMENT_AUTHORS) + assert reloaded.content_type == CT.PML_COMMENT_AUTHORS + names = [a.name for a in reloaded.iter_authors()] + assert names == ["Ada"] + + +# -- parts: legacy + modern comments parts ------------------------------------- + + +class DescribeCommentsPart: + def it_creates_an_empty_legacy_part_via_new(self): + prs = Presentation() + part = CommentsPart.new(prs.part.package, PackURI("/ppt/comments/comment1.xml")) + assert part.content_type == CT.PML_COMMENTS + # empty + assert len(part._element.cm_lst) == 0 + + +class DescribeModernCommentsPart: + def it_creates_an_empty_modern_part_via_new(self): + prs = Presentation() + part = ModernCommentsPart.new( + prs.part.package, PackURI("/ppt/comments/modernComment_slide1.xml") + ) + assert part.content_type == CT.PML_THREADED_COMMENTS + + +# -- content-type registry ----------------------------------------------------- + + +class DescribeContentTypePartClassMap: + def it_maps_the_two_legacy_part_classes(self): + assert PartFactory.part_type_for[CT.PML_COMMENT_AUTHORS] is CommentAuthorsPart + assert PartFactory.part_type_for[CT.PML_COMMENTS] is CommentsPart + + def it_maps_the_modern_threaded_part_class(self): + assert PartFactory.part_type_for[CT.PML_THREADED_COMMENTS] is ModernCommentsPart + + +# -- Wave 2: Slide.comments collection + Comment/replies proxy ----------------- + + +def _blank_slide(): + """A fresh presentation + one blank slide (layout 6 = Blank).""" + prs = Presentation() + slide = prs.slides.add_slide(prs.slide_layouts[6]) + return prs, slide + + +class DescribeSlideComments: + def it_exposes_an_empty_comments_collection_without_mutating(self): + _prs, slide = _blank_slide() + assert hasattr(slide, "comments") + assert len(slide.comments) == 0 + assert list(slide.comments) == [] + # reading must not have created the modern-comments part (ISC-21 + # spirit: no side effects on read) + assert slide.part.has_modern_comments is False + + def it_add_returns_a_Comment_and_increments_len(self): + from pptx.comments import Comment + + _prs, slide = _blank_slide() + comment = slide.comments.add("Hello", "Ada") + assert isinstance(comment, Comment) + assert comment.text == "Hello" + assert len(slide.comments) == 1 + + def it_creates_the_part_and_rel_on_first_add(self): + _prs, slide = _blank_slide() + assert slide.part.has_modern_comments is False + slide.comments.add("first", "Ada") + assert slide.part.has_modern_comments is True + # same single part returned on every access + assert slide.part.modern_comments_part is slide.part.modern_comments_part + + def it_dedupes_the_author_on_the_modern_authors_part(self): + # ISC-5/ISC-21: two comments by the same author name -> one entry on + # the MODERN authors part (NOT the legacy commentAuthors.xml — that + # was the issue-#25 repair-dialog bug; the old assertion encoded it). + prs, slide = _blank_slide() + slide.comments.add("a", "Ada") + slide.comments.add("b", "Ada") + authors = list(prs.part.package.presentation_part.authors_part.iter_authors()) + assert [a.name for a in authors] == ["Ada"] + # and the legacy part was never created for a modern-only deck + assert prs.part.package.presentation_part.has_comment_authors is False + + def it_iterates_comments_in_document_order(self): + _prs, slide = _blank_slide() + slide.comments.add("one", "Ada") + slide.comments.add("two", "Babbage") + slide.comments.add("three", "Ada") + assert [c.text for c in slide.comments] == ["one", "two", "three"] + + def it_remove_decrements_len(self): + _prs, slide = _blank_slide() + slide.comments.add("keep", "Ada") + target = slide.comments.add("drop", "Ada") + assert len(slide.comments) == 2 + slide.comments.remove(target) + assert len(slide.comments) == 1 + assert [c.text for c in slide.comments] == ["keep"] + + def it_leaves_valid_rels_when_the_last_comment_is_removed(self): + # ISC-21 anti-criterion: removing the last comment must NOT drop + # the part/relationship and the file must still reopen cleanly. + prs, slide = _blank_slide() + only = slide.comments.add("solo", "Ada") + slide.comments.remove(only) + assert len(slide.comments) == 0 + assert slide.part.has_modern_comments is True # rel intact + stream = io.BytesIO() + prs.save(stream) + stream.seek(0) + prs2 = Presentation(stream) + assert len(prs2.slides[0].comments) == 0 + + def it_round_trips_text_and_author_through_save_reopen(self): + # ISC-19: save -> BytesIO -> reopen preserves text + author. + prs, slide = _blank_slide() + slide.comments.add("first body", "Ada Lovelace") + slide.comments.add("second body", "Charles Babbage") + stream = io.BytesIO() + prs.save(stream) + stream.seek(0) + reloaded = list(Presentation(stream).slides[0].comments) + assert [c.text for c in reloaded] == ["first body", "second body"] + assert [c.author for c in reloaded] == ["Ada Lovelace", "Charles Babbage"] + + +class DescribeCommentReplies: + def it_add_threads_a_reply_and_preserves_order(self): + from pptx.comments import CommentReply + + _prs, slide = _blank_slide() + comment = slide.comments.add("question?", "Ada") + r1 = comment.replies.add("answer 1", "Babbage") + comment.replies.add("answer 2", "Ada") + assert isinstance(r1, CommentReply) + assert len(comment.replies) == 2 + assert [r.text for r in comment.replies] == ["answer 1", "answer 2"] + + def it_does_not_detach_the_parent_comment_on_reply_add(self): + # ISC-27 anti-criterion: adding a reply must not re-parent or drop + # the top-level comment. + _prs, slide = _blank_slide() + comment = slide.comments.add("parent", "Ada") + comment.replies.add("child", "Babbage") + assert len(slide.comments) == 1 + assert slide.comments[0].text == "parent" + + def it_round_trips_replies_through_save_reopen(self): + prs, slide = _blank_slide() + comment = slide.comments.add("top", "Ada") + comment.replies.add("reply one", "Babbage") + comment.replies.add("reply two", "Ada") + stream = io.BytesIO() + prs.save(stream) + stream.seek(0) + reloaded = list(Presentation(stream).slides[0].comments) + assert len(reloaded) == 1 + replies = list(reloaded[0].replies) + assert [r.text for r in replies] == ["reply one", "reply two"] + assert [r.author for r in replies] == ["Babbage", "Ada"] + + +class DescribeCommentProxyFields: + def it_resolves_author_text_and_tz_aware_created_at(self): + from datetime import datetime + + _prs, slide = _blank_slide() + comment = slide.comments.add("body text", "Grace Hopper") + assert comment.author == "Grace Hopper" + assert comment.text == "body text" + assert isinstance(comment.created_at, datetime) + assert comment.created_at.tzinfo is not None # tz-aware + + def it_reports_None_anchor_position_when_unanchored(self): + _prs, slide = _blank_slide() + comment = slide.comments.add("floating", "Ada") + assert comment.anchor_position is None + + def it_resolves_anchor_position_to_shape_left_top_when_anchored(self): + from pptx.enum.shapes import MSO_SHAPE + from pptx.util import Emu + + prs, slide = _blank_slide() + shape = slide.shapes.add_shape( + MSO_SHAPE.RECTANGLE, Emu(914400), Emu(457200), Emu(100), Emu(200) + ) + comment = slide.comments.add("on the box", "Ada", anchor=shape) + assert comment.anchor_position == (914400, 457200) + # round-trips + stream = io.BytesIO() + prs.save(stream) + stream.seek(0) + reloaded = list(Presentation(stream).slides[0].comments)[0] + assert reloaded.anchor_position == (914400, 457200) + + def it_returns_None_author_for_an_unknown_author_guid(self): + _prs, slide = _blank_slide() + comment = slide.comments.add("x", "Ada") + comment._cm.authorId = "{00000000-0000-0000-0000-000000000000}" + assert comment.author is None + + +# -- Wave 3: SF6 resolve / SF7 Shape.comments / SF8 legacy coexistence --------- + + +class DescribeCommentResolve: + """SF6 (ISC-33..37): Comment.resolve() / Comment.resolved.""" + + def it_defaults_resolved_to_False(self): + _prs, slide = _blank_slide() + comment = slide.comments.add("open question", "Ada") + assert comment.resolved is False + + def it_marks_a_thread_resolved(self): + _prs, slide = _blank_slide() + comment = slide.comments.add("please review", "Ada") + comment.resolve() + assert comment.resolved is True + + def it_can_reopen_a_resolved_thread(self): + _prs, slide = _blank_slide() + comment = slide.comments.add("review me", "Ada") + comment.resolve() + comment.reopen() + assert comment.resolved is False + + def it_round_trips_the_resolved_flag(self): + # ISC-35: save -> reopen preserves resolution state. + prs, slide = _blank_slide() + a = slide.comments.add("resolved one", "Ada") + slide.comments.add("still open", "Babbage") + a.resolve() + stream = io.BytesIO() + prs.save(stream) + stream.seek(0) + reloaded = list(Presentation(stream).slides[0].comments) + assert [c.resolved for c in reloaded] == [True, False] + + def it_raises_when_resolving_a_legacy_backed_comment(self): + # ISC-37 anti: the legacy schema has no resolution concept. + # Decision: raise (not silent no-op) so callers can't believe a + # legacy thread was resolved when the file format cannot record it. + prs, slide = _legacy_plus_modern_deck() + legacy = next(c for c in slide.comments if c.is_legacy) + with pytest.raises(TypeError): + legacy.resolve() + assert legacy.resolved is False + + +class DescribeShapeComments: + """SF7 (ISC-38..41): per-shape comment filter.""" + + def it_returns_an_empty_collection_for_a_shape_with_no_comments(self): + from pptx.enum.shapes import MSO_SHAPE + from pptx.util import Emu + + _prs, slide = _blank_slide() + shape = slide.shapes.add_shape(MSO_SHAPE.RECTANGLE, Emu(1), Emu(2), Emu(3), Emu(4)) + assert hasattr(shape, "comments") + assert list(shape.comments) == [] + assert len(shape.comments) == 0 + + def it_yields_only_comments_anchored_to_that_shape(self): + from pptx.enum.shapes import MSO_SHAPE + from pptx.util import Emu + + _prs, slide = _blank_slide() + a = slide.shapes.add_shape(MSO_SHAPE.RECTANGLE, Emu(10), Emu(20), Emu(30), Emu(40)) + b = slide.shapes.add_shape(MSO_SHAPE.OVAL, Emu(50), Emu(60), Emu(70), Emu(80)) + slide.comments.add("on A one", "Ada", anchor=a) + slide.comments.add("on B", "Babbage", anchor=b) + slide.comments.add("on A two", "Ada", anchor=a) + slide.comments.add("floating", "Grace") + assert [c.text for c in a.comments] == ["on A one", "on A two"] + + def it_does_not_leak_a_comment_across_shapes(self): + # ISC-40: a comment anchored to shape A must not appear under B. + from pptx.enum.shapes import MSO_SHAPE + from pptx.util import Emu + + _prs, slide = _blank_slide() + a = slide.shapes.add_shape(MSO_SHAPE.RECTANGLE, Emu(10), Emu(20), Emu(30), Emu(40)) + b = slide.shapes.add_shape(MSO_SHAPE.OVAL, Emu(50), Emu(60), Emu(70), Emu(80)) + slide.comments.add("only on A", "Ada", anchor=a) + assert [c.text for c in a.comments] == ["only on A"] + assert list(b.comments) == [] + + def it_round_trips_the_per_shape_filter(self): + from pptx.enum.shapes import MSO_SHAPE + from pptx.util import Emu + + prs, slide = _blank_slide() + a = slide.shapes.add_shape(MSO_SHAPE.RECTANGLE, Emu(11), Emu(22), Emu(33), Emu(44)) + slide.comments.add("anchored", "Ada", anchor=a) + stream = io.BytesIO() + prs.save(stream) + stream.seek(0) + slide2 = Presentation(stream).slides[0] + shape2 = next(s for s in slide2.shapes if s.shape_id == a.shape_id) + assert [c.text for c in shape2.comments] == ["anchored"] + + +def _legacy_plus_modern_deck(): + """Build a deck that already has a LEGACY comment, then add a + MODERN threaded comment to the same slide. + + The legacy comment is hand-injected into a saved package by editing the + zip directly (our public ``.add()`` only writes modern), mirroring how + the rest of this module builds raw-xml fixtures. Returns + ``(prs, slide)`` of the *reopened* package with both families present. + """ + import zipfile + + prs0 = Presentation() + prs0.slides.add_slide(prs0.slide_layouts[6]) + base = io.BytesIO() + prs0.save(base) + base.seek(0) + + legacy_authors = ( + '\n' + '' + '' + "" + ) + legacy_comments = ( + '\n' + '' + '' + '' + "Legacy feedback" + "" + "" + ) + + src = zipfile.ZipFile(base, "r") + names = src.namelist() + ct = src.read("[Content_Types].xml").decode("utf-8") + ct = ct.replace( + "", + '' + '' + "", + ) + pres_rels_name = "ppt/_rels/presentation.xml.rels" + pres_rels = src.read(pres_rels_name).decode("utf-8") + pres_rels = pres_rels.replace( + "", + '' + "", + ) + slide_rels_name = "ppt/slides/_rels/slide1.xml.rels" + slide_rels = src.read(slide_rels_name).decode("utf-8") + slide_rels = slide_rels.replace( + "", + '' + "", + ) + + patched = { + "[Content_Types].xml": ct, + pres_rels_name: pres_rels, + slide_rels_name: slide_rels, + "ppt/commentAuthors.xml": legacy_authors, + "ppt/comments/comment1.xml": legacy_comments, + } + out = io.BytesIO() + dst = zipfile.ZipFile(out, "w", zipfile.ZIP_DEFLATED) + for name in names: + dst.writestr(name, patched.get(name, src.read(name))) + for name, data in patched.items(): + if name not in names: + dst.writestr(name, data) + src.close() + dst.close() + out.seek(0) + + prs = Presentation(out) + slide = prs.slides[0] + slide.comments.add("Modern reply-era feedback", "Modern Mary") + return prs, slide + + +class DescribeLegacyModernCoexistence: + """SF8 (ISC-42..46, ISC-67): legacy + modern coexist.""" + + def it_keeps_the_legacy_part_when_a_modern_comment_is_added(self): + # ISC-43: adding a modern comment must NOT delete commentsN.xml. + prs, slide = _legacy_plus_modern_deck() + partnames = {p.partname for p in prs.part.package.iter_parts()} + assert PackURI("/ppt/comments/comment1.xml") in partnames + + def it_enumerates_both_families_after_save_and_reopen(self): + # ISC-44: slide.comments must read legacy AND modern + # and present both. ISC-67: legacy never silently dropped. + prs, _slide = _legacy_plus_modern_deck() + stream = io.BytesIO() + prs.save(stream) + stream.seek(0) + comments = list(Presentation(stream).slides[0].comments) + texts = sorted(c.text for c in comments) + assert texts == ["Legacy feedback", "Modern reply-era feedback"] + + def it_exposes_is_legacy_to_distinguish_the_families(self): + prs, slide = _legacy_plus_modern_deck() + by_text = {c.text: c for c in slide.comments} + assert by_text["Legacy feedback"].is_legacy is True + assert by_text["Modern reply-era feedback"].is_legacy is False + + def it_does_not_mangle_legacy_author_ids_on_modern_write(self): + # ISC-46 anti: modern write must not rewrite legacy /author ids. + prs, _slide = _legacy_plus_modern_deck() + stream = io.BytesIO() + prs.save(stream) + stream.seek(0) + import zipfile + + zf = zipfile.ZipFile(stream, "r") + legacy_xml = zf.read("ppt/comments/comment1.xml").decode("utf-8") + authors_xml = zf.read("ppt/commentAuthors.xml").decode("utf-8") + zf.close() + assert 'authorId="0"' in legacy_xml + assert 'idx="1"' in legacy_xml + assert 'id="0"' in authors_xml + assert 'name="Legacy Larry"' in authors_xml + + def it_resolves_the_legacy_author_name(self): + prs, slide = _legacy_plus_modern_deck() + legacy = next(c for c in slide.comments if c.is_legacy) + assert legacy.author == "Legacy Larry" + + def it_treats_a_legacy_comment_replies_as_empty_read_only(self): + # legacy schema has no reply thread — SF4 documented legacy behavior. + prs, slide = _legacy_plus_modern_deck() + legacy = next(c for c in slide.comments if c.is_legacy) + assert len(legacy.replies) == 0 + assert list(legacy.replies) == [] + + +class DescribeThreadedCommentBodyPrRegression: + """Regression: MUST carry (issue #25 silent-drop). + + The add-path builds txBody via get_or_add_txBody() (a bare element); + a CT_TextBody without is schema-malformed and PowerPoint + SILENTLY drops the comment (no repair dialog, comment just absent — + caught only by maintainer visual review). Trinity was green while + every comment was invisible. This locks the fix. + """ + + def _modern_xml(self): + import io + import zipfile + + from pptx import Presentation + + prs = Presentation() + s = prs.slides.add_slide(prs.slide_layouts[1]) + c = s.comments.add("Body check", author="Rev") + c.replies.add("reply body check", author="Rev2") + buf = io.BytesIO() + prs.save(buf) + buf.seek(0) + z = zipfile.ZipFile(buf) + name = next(n for n in z.namelist() if "modernComment" in n) + return z.read(name).decode() + + def it_emits_bodyPr_in_every_comment_txBody(self): + import re + + xml = self._modern_xml() + bodies = re.findall(r"(.*?)", xml, re.S) + assert bodies, "expected at least one " + assert all("bodyPr" in b for b in bodies), ( + "every threaded-comment/reply txBody must contain " + "or PowerPoint silently drops the comment" + ) + + def it_places_bodyPr_before_the_paragraph(self): + import re + + xml = self._modern_xml() + body = re.search(r"(.*?)", xml, re.S).group(1) + assert body.index("bodyPr") < body.index(" must precede (CT_TextBody child order)" + ) + + +class DescribeThreadedCommentPowerPointContract: + """Regression: the modern-comment OOXML must match PowerPoint ground truth. + + Captured 2026-05-17 from a threaded comment PowerPoint for Mac itself + authored+saved. Three string axes plus a slide-binding element were + inferred wrong in Waves 1-2 (content type ``threadedComments+xml``, + reltypes ``threadedComment``/``threadedCommentAuthors``, and a missing + ````). With any of them wrong PowerPoint silently fails to + load/bind the part → empty Comments pane while the test trinity is + green. This locks every axis to the PowerPoint-emitted contract. + """ + + def it_uses_the_powerpoint_content_type_and_reltypes(self): + assert CT.PML_THREADED_COMMENTS == "application/vnd.ms-powerpoint.comments+xml" + assert RT.THREADED_COMMENT == ( + "http://schemas.microsoft.com/office/2018/10/relationships/comments" + ) + assert RT.AUTHORS == "http://schemas.microsoft.com/office/2018/10/relationships/authors" + + def _round_trip_zip(self): + prs = Presentation() + s = prs.slides.add_slide(prs.slide_layouts[1]) + s.comments.add("Looks great! Ship it.", author="Alex Reviewer") + buf = io.BytesIO() + prs.save(buf) + buf.seek(0) + import zipfile + + return zipfile.ZipFile(buf), s.slide_id + + def it_stamps_the_powerpoint_content_type_in_content_types(self): + z, _ = self._round_trip_zip() + ctypes = z.read("[Content_Types].xml").decode() + assert "application/vnd.ms-powerpoint.comments+xml" in ctypes + assert "threadedComments+xml" not in ctypes + + def it_relates_slide_to_part_and_presentation_to_authors_by_pp_reltype(self): + z, _ = self._round_trip_zip() + slide_rels = z.read("ppt/slides/_rels/slide1.xml.rels").decode() + prs_rels = z.read("ppt/_rels/presentation.xml.rels").decode() + assert "/2018/10/relationships/comments" in slide_rels + assert "/2018/10/relationships/threadedComment" not in slide_rels + assert "/2018/10/relationships/authors" in prs_rels + assert "threadedCommentAuthors" not in prs_rels + + def it_binds_each_comment_to_its_slide_via_sldMkLst(self): + import re + + z, slide_id = self._round_trip_zip() + name = next(n for n in z.namelist() if "modernComment" in n) + xml = z.read(name).decode() + # present with and + assert "sldMkLst" in xml, "every needs a slide binding" + assert "docMk" in xml + assert "sldMk" in xml + m = re.search(r']*sldId="(\d+)"', xml) + assert m is not None, " must carry the slide's sldId" + assert int(m.group(1)) == slide_id, ( + "sldMk/@sldId must equal the slide's /@id (%d) so " + "PowerPoint binds the comment to the right slide" % slide_id + ) + + def it_places_sldMkLst_before_txBody_in_child_order(self): + z, _ = self._round_trip_zip() + name = next(n for n in z.namelist() if "modernComment" in n) + xml = z.read(name).decode() + assert xml.index("sldMkLst") < xml.index("txBody"), ( + " must precede (PowerPoint child order)" + ) + + def it_keeps_exactly_one_sldMkLst_when_set_twice(self): + # Cato low-finding lock: set_slide_marker must REPLACE, not prepend a + # second , if invoked again on the same comment element. + prs = Presentation() + s = prs.slides.add_slide(prs.slide_layouts[1]) + c = s.comments.add("once", author="Rev") + c._cm.set_slide_marker(s.slide_id) + c._cm.set_slide_marker(s.slide_id) + from pptx.oxml.ns import qn + + assert len(c._cm.findall(qn("pc:sldMkLst"))) == 1, ( + "set_slide_marker must be idempotent (exactly one )" + ) + + def it_places_replyLst_before_txBody_per_ms_pptx_sequence(self): + # [MS-PPTX] CT_Comment sequence: EG_CommentAnchor, pos?, replyLst?, + # EG_CommentProperties(txBody, extLst). replyLst MUST precede txBody. + # Emitting replyLst AFTER txBody makes PowerPoint render the parent + # comment but SILENTLY DROP every reply (issue #25, the reply defect). + prs = Presentation() + s = prs.slides.add_slide(prs.slide_layouts[1]) + c = s.comments.add("parent body", author="Rev") + c.replies.add("a reply body", author="Rev2") + buf = io.BytesIO() + prs.save(buf) + buf.seek(0) + import zipfile + + z = zipfile.ZipFile(buf) + name = next(n for n in z.namelist() if "modernComment" in n) + xml = z.read(name).decode() + assert "replyLst" in xml, "expected a for the reply" + assert "a reply body" in xml, "reply text must serialize" + assert xml.index("replyLst") < xml.index("txBody"), ( + " must precede per [MS-PPTX] " + "CT_Comment sequence, or PowerPoint drops every reply" + ) + + def it_keeps_reply_txBody_with_bodyPr_after_reorder(self): + # Guard the reorder didn't regress the reply body shape. + prs = Presentation() + s = prs.slides.add_slide(prs.slide_layouts[1]) + c = s.comments.add("p", author="Rev") + c.replies.add("r", author="Rev2") + buf = io.BytesIO() + prs.save(buf) + buf.seek(0) + import re + import zipfile + + z = zipfile.ZipFile(buf) + name = next(n for n in z.namelist() if "modernComment" in n) + xml = z.read(name).decode() + reply = re.search(r"", xml, re.S).group(0) + assert "bodyPr" in reply + assert reply.index("bodyPr") < reply.index(" None: - global ok - print((" PASS " if cond else " FAIL ") + label) - ok = ok and cond - - -print("=== issue #19 UAT ===") - -# ---------- SF2: save_as_potx (and SF1 read-back) ---------- -prs = Presentation() -before_ct = prs.part.content_type -prs.save_as_potx(POTX) -with zipfile.ZipFile(POTX) as z: - ct_xml = z.read("[Content_Types].xml").decode() -check("SF2 save_as_potx writes template content-type", - "presentationml.template.main+xml" in ct_xml) -check("SF2 in-memory package content-type unmutated", - prs.part.content_type == before_ct) -# SF1: the .potx we just wrote re-opens (was a hard ValueError before the fix) -reopened = Presentation(POTX) -check("SF1 Presentation('.potx') opens without error", reopened is not None) -check("SF1 reopened .potx exposes masters", len(reopened.slide_masters) >= 1) - -# ---------- The layouts deck ---------- -prs = Presentation() -master = prs.slide_masters[0] - -# SF3: add_layout("Three Columns") — the issue's headline acceptance check -n_before = len(master.slide_layouts) -three_col = master.slide_layouts.add_layout(name="Three Columns") -check("SF3 add_layout increments layout count by 1", - len(master.slide_layouts) == n_before + 1) -check("SF3 new layout name == 'Three Columns'", three_col.name == "Three Columns") - -# SF6: programmatically add three body placeholders → an actual 3-column layout -for col, left in enumerate((Inches(0.4), Inches(4.7), Inches(9.0))): - three_col.placeholders.add( - idx=10 + col, - ph_type=PP_PLACEHOLDER.BODY, - name="Column %d" % (col + 1), - left=left, - top=Inches(1.8), - width=Inches(4.0), - height=Inches(4.8), - ) -check("SF6 three placeholders added to layout", - len(three_col.placeholders) >= 3) - -# SF5: author a shape directly on the master (visible on every slide) -wm = master.shapes.add_shape( - MSO_SHAPE.ROUNDED_RECTANGLE, Inches(0.2), Inches(0.1), Inches(3.0), Inches(0.5) -) -wm.text_frame.text = "MASTER BANNER (SF5)" -wm.text_frame.paragraphs[0].runs[0].font.size = Pt(14) -check("SF5 shape authored on master", wm is not None) - -# SF4: copy_from — duplicate the Three Columns layout -copy = master.slide_layouts.copy_from(three_col) -check("SF4 copy_from duplicates shape/placeholder count", - len(list(copy.shapes)) == len(list(three_col.shapes))) -check("SF4 copy_from leaves source untouched", - len(list(three_col.shapes)) > 0) - -# A real slide built on the new "Three Columns" layout (so it renders visibly) -slide = prs.slides.add_slide(three_col) -for i, ph in enumerate(slide.placeholders): - try: - ph.text = "Column %d content" % (i + 1) - except Exception: - pass - -# SF9: get_layout by id round-trips to the same object -idLst = master._element.get_or_add_sldLayoutIdLst() -some_id = next((e.id for e in idLst.sldLayoutId_lst if e.id is not None), None) -if some_id is not None: - check("SF9 get_layout(id) returns a layout", - master.get_layout(some_id) is not None) -check("SF9 get_layout(bad id) returns None (no raise)", - master.get_layout(987654321) is None) - -# SF7: reassign the slide to a different layout (cross-master mechanism) -slide.slide_layout = copy -check("SF7 slide_layout setter repoints layout", - slide.slide_layout.name == copy.name) - -# SF8: chart into a chart placeholder — the SF6→SF8 composition: -# build a CHART placeholder programmatically, then drop a chart into it. -chart_done = False -probe = Presentation() -pm = probe.slide_masters[0] -chart_layout = pm.slide_layouts.add_layout(name="Chart Slot") -chart_layout.placeholders.add( - idx=10, - ph_type=PP_PLACEHOLDER.CHART, - left=Inches(1.0), - top=Inches(1.2), - width=Inches(8.0), - height=Inches(4.5), -) -cslide = probe.slides.add_slide(chart_layout) -chart_ph = next( - p for p in cslide.placeholders - if p.placeholder_format.type == PP_PLACEHOLDER.CHART -) -cd = CategoryChartData() -cd.categories = ["Q1", "Q2", "Q3"] -cd.add_series("Revenue", (10, 24, 18)) -gf = chart_ph.insert_chart(XL_CHART_TYPE.COLUMN_CLUSTERED, cd) -chart_done = bool(gf.has_chart) -check("SF8 insert_chart into a CHART placeholder (SF6+SF8)", chart_done) -if chart_done: - probe.save(PPTX.replace(".pptx", "_chart.pptx")) - -prs.save(PPTX) - -# Final structural round-trip of the main deck -rt = Presentation(PPTX) -check("ALL round-trip: layouts deck reopens with 'Three Columns'", - rt.slide_masters[0].slide_layouts.get_by_name("Three Columns") is not None) - -print("ARTIFACTS:", POTX, PPTX) -print("RESULT:", "PASS" if ok else "FAIL") -sys.exit(0 if ok else 1)