Conversation
Word's multi-level list styles ("List Bullet 2", "List Number 2", ...)
each link to their own numbering definition at w:ilvl 0 and differ from
"List Bullet" / "List Number" only by a deeper indentation. The backend
nested lists by w:ilvl alone, so each of these styles started a new
top-level list, and the outer list did not continue after it.
When an item of another numId follows an open list's item, compare their
effective left indentation: the paragraph's own w:ind, else its numbering
level's, else its style's basedOn chain (w:left or w:start). A deeper
item opens its list under that item. A later item of another numId that
is indented no deeper closes the nested list and is placed as if it came
right after that item. Lists at the same indentation stay siblings, as
before. When a blank paragraph closes a nested list, the group kept for
reuse is the outer list's.
Signed-off-by: wenqiw777 <wenqiw@umich.edu>
|
✅ DCO Check Passed Thanks @wenqiw777, all your commits are properly signed off. 🎉 |
Merge Protections🟢 Merge protection satisfied — ready to merge. Show 1 satisfied protection🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
dolfim-ibm
left a comment
dolfim-ibm
left a comment
There was a problem hiding this comment.
The nesting fix looks good. One request on the style-chain walk: getattr(style, "base_style", None) probes the attribute instead of relying on python-docx's types, which AGENTS.md asks us to avoid.
In python-docx, base_style is defined on CharacterStyle. ParagraphStyle and _TableStyle inherit it; _NumberingStyle doesn't. So an isinstance check expresses exactly the "malformed chain hops to a numbering style" case the comment describes. A small shared iterator would replace the probe and the depth bookkeeping:
from collections.abc import Iterator
from docx.styles.style import BaseStyle, CharacterStyle, ParagraphStyle # add CharacterStyle
def _iter_style_chain(self, style: BaseStyle | None) -> Iterator[CharacterStyle]:
"""Yield ``style`` and its ``basedOn`` ancestors.
Stops at a style type without ``base_style`` (e.g. a numbering style
reached through a malformed chain) and at ``_MAX_STYLE_INHERITANCE_DEPTH``
to guard against cycles.
"""
depth = 0
while isinstance(style, CharacterStyle) and depth < self._MAX_STYLE_INHERITANCE_DEPTH:
yield style
style = style.base_style
depth += 1With that, _get_list_left_indent becomes:
left = _left(paragraph._p)
if left is None:
left = _left(self._get_level_element(numid, ilvl))
if left is None:
for style in self._iter_style_chain(paragraph.style):
left = _left(style.element)
if left is not None:
break
return left or 0The same walk is already copied with getattr on main (_style_numbering, _is_code_style, _effective_style_font, the bold lookup in _get_format_from_run, and the base_style check in _get_label_and_level). Those aren't introduced here. Moving them to the helper in this PR would be welcome but is optional; at minimum, the new walk shouldn't add a fourth copy.
| left = _left(style.element) | ||
| # A malformed basedOn chain can hop to a style type that lacks | ||
| # base_style; getattr keeps the walk safe. | ||
| style = getattr(style, "base_style", None) |
There was a problem hiding this comment.
Instead of probing with getattr, iterate with an isinstance(style, CharacterStyle) guard. See the _iter_style_chain proposal in the review summary.
…iterator _get_list_left_indent probed base_style with getattr while walking a list paragraph's basedOn chain. In python-docx, base_style is defined on CharacterStyle, which ParagraphStyle and _TableStyle inherit and _NumberingStyle does not, so an isinstance check expresses the "malformed chain hops to a numbering style" case directly, without probing the attribute. Add _iter_style_chain, which yields a style and its basedOn ancestors, stops at a style type without base_style and caps the walk at _MAX_STYLE_INHERITANCE_DEPTH, and use it for the style step of _get_list_left_indent. Behavior is unchanged. Signed-off-by: wenqiw777 <wenqiw@umich.edu>
|
Thanks, that reads much better. 314f373 adds |
This branch has not been deployed
2 participants
Issue resolved by this Pull Request:
Resolves #4464
Word's multi-level list styles ("List Bullet 2", "List Number 2", ...) each link to their own numbering definition at
w:ilvl0 and differ from "List Bullet" / "List Number" only by a deeper indentation. The backend nested lists byw:ilvlalone, so each of these styles started a new top-level list, and the outer list did not continue after it.When an item of another
numIdfollows an open list's item, the backend now compares their effective left indentation: the paragraph's ownw:ind, else its numbering level's, else its style'sbasedOnchain (w:leftorw:start).numIdthat is indented no deeper closes the nested list and is placed as if it came right after that item, so the outer list continues.The nested-list state is saved and restored around table cells and reset for each header/footer part. When a blank paragraph closes a nested list, the group kept for reuse is the outer list's.
_get_numId_and_ilvlis untouched, so this does not overlap with #4457.Reproduction from the issue, before:
After:
Tests: five tests in
tests/test_backend_msword_lists.pycover nesting for bullets and numbers, lists at the same indentation staying siblings, returning to the outer list, a blank paragraph inside a nested list, and the indentation precedence. All five fail on main. The 36 DOCX fixtures give identical Markdown, text and JSON output, so no groundtruth changes. A 6000-paragraph list document converts about 2% slower, from one extra numbering lookup per list item.One choice to flag: "deeper" has no tolerance, so two unrelated adjacent lists whose indents differ by a few twips will nest, the same way Word shows them. I can add a minimum difference if you prefer.
Checklist: