Skip to content

Keep comment anchors when assigning paragraph.text - #1605

Open
gyanu2507 wants to merge 2 commits into
python-openxml:masterfrom
gyanu2507:paragraph-text-keeps-comments
Open

Keep comment anchors when assigning paragraph.text#1605
gyanu2507 wants to merge 2 commits into
python-openxml:masterfrom
gyanu2507:paragraph-text-keeps-comments

Conversation

@gyanu2507

@gyanu2507 gyanu2507 commented Sep 8, 2026

Copy link
Copy Markdown

Setting paragraph.text runs clear(), which dropped w:commentRangeStart / w:commentRangeEnd and the comment reference run. The comment body stayed in comments.xml with nothing pointing at it.

Collect the previous comment ids and re-mark them on the replacement run. The range then covers that whole run, not the original word span.

This is the comment half of #1604. Footnote references and tracked-change markup still drop on clear().

clear() dropped commentRangeStart/End and the comment reference run, so
comments.xml kept a comment with nothing pointing at it.
@Dmitry-Kov

Copy link
Copy Markdown

Thanks for picking this up. I tested the logic in this PR against python-docx 1.2.0 and it does fix the comment case: before 3 markers / 1 comment, after paragraph.text = ... again 3 markers / 1 comment, with w:commentRangeStart, w:commentRangeEnd and the reference run correctly wrapped around the new run.

Three notes, in case they're useful before review.

1. Footnote references and tracked changes are still dropped. Running the same logic on a document that has a footnote reference in the paragraph:

footnoteReference: 2 -> 1
FTN002  footnote id=1 is defined but never referenced - the footnote no longer appears

w:ins / w:del wrapping the runs go the same way, since clear() removes them and nothing re-emits them. So this closes the comment half of #1604 but not the whole issue — might be worth either widening the PR or narrowing the "Fixes" to a note that footnotes and revisions remain open.

2. The comment range widens to the whole paragraph. If the comment was anchored to a few words inside the paragraph, it now covers all of the replacement text. I think that's the only sensible behaviour for a full-text replacement, and it matches what Word does when you retype a commented passage, but it's worth stating in the docstring rather than leaving as an implicit side effect — otherwise someone will read "stay anchored" as "anchored to the same span".

3. Pre-existing bug this PR surfaces: the reference run gets rStyle w:val="CommentReference", but add_comment() never adds that style to styles.xml, so the reference mark falls back to the run's default formatting in both Word and LibreOffice with no warning. This comes from Run.mark_comment_range, not from your change — it's already there in 1.2.0 whenever you add a comment. The definition Word writes is:

<w:style w:type="character" w:styleId="CommentReference">
  <w:name w:val="annotation reference"/>
  <w:basedOn w:val="DefaultParagraphFont"/>
  <w:uiPriority w:val="99"/>
  <w:semiHidden/><w:unhideWhenUsed/>
  <w:rPr><w:sz w:val="16"/><w:szCs w:val="16"/></w:rPr>
</w:style>

Happy to file that as a separate issue if you'd rather keep this PR focused. (Same gap existed in adeu and was fixed in their 3.0.3, for reference.)

Checked with ooxml-integrity against the source document, python-docx 1.2.0, Python 3.9.6, macOS.

A full-text assignment cannot keep the original word span, and footnotes
plus tracked changes still drop. Say that on the property so it is not
read as preserving the original anchor.
@gyanu2507

Copy link
Copy Markdown
Author

Thanks for checking this against 1.2.0.

Kept the PR on comments. Footnotes and w:ins/w:del still go through clear(), and I don't want to call #1604 closed. The description now says that.

The docstring now states the range covers the whole replacement run.

The missing CommentReference style is outside this change, as you said. A separate issue is the right place if you want to file it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants