Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(text): Render list items correctly #1137

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix(text): Render list items correctly #1137

wants to merge 5 commits into from

Conversation

kesara
Copy link
Member

@kesara kesara commented Jul 9, 2024 •
edited
Loading

Fixes #1135
Fixes #1138

See changes made in 3712523 for context.

This change can affect if ol is used inside a li as the first element and the li element is inside another ol element.

I could not find any published RFCs with XML that match the above condition.

These are the RFCs that I found that use ol as the first element in li:

  • RFC8679
  • RFC8687

I didn't notice any changes in text output due to this change. But there are a few minor changes (with spacing) due to changes over the years.

rjsparks
rjsparks reviewed Jul 9, 2024
View reviewed changes
xml2rfc/writers/text.py Outdated Show resolved Hide resolved
@rjsparks
Copy link
Member

rjsparks commented Jul 9, 2024

I think it's worth trying to change things such that generatiing

   4.  4.1  Sublist with '%p' parent counter element in the list
            counter, item one

       4.2  Item two.

       4.3  Item three.

we generate

   4. 

       4.1  Sublist with '%p' parent counter element in the list
            counter, item one

       4.2  Item two.

       4.3  Item three.

Unless that breaks the intended fix for the rfc-to-be.

@cabo
Copy link
Contributor

cabo commented Jul 9, 2024

What is better about that superfluous empty line?

(You can probably have one if you want with <br>)

@rjsparks
Copy link
Member

rjsparks commented Jul 9, 2024 •
edited
Loading

Look at it in context with the rest of the nested <ol> above it in the test document:

xml2rfc/tests/valid/elements.bom.text

Lines 366 to 458 in fdb6fb8

4. Ordered list with sub-elements
1. Ordered list, first bullet. Indent="6".
2. Ordered list, second bullet. Indent="6". Text only. Octavio
fuit, cum illam severitatem in eo non arbitrantur. erunt etiam,
et ii quidem eruditi Graecis litteris, contemnentes Latinas,
qui se plane Graecum dici velit, ut a Scaevola est praetore
salutatus Athenis Albucius. In physicis plurimum posuit. ea
scientia et verborum vis et natura orationis et consequentium
repugnantiumve ratio potest perspici.
3. Ordered list, third bullet, element t. Quid? si nos non
interpretum fungimur munere, sed tuemur ea, quae audiebamus,
conferebamus, neque erat umquam controversia, quid ego
intellegerem, sed quid probarem. Extremum autem esse bonorum eum
voluptate vivere. Sic faciam igitur, inquit: unam rem explicabo,
eamque maximam, de physicis alias, et quidem locis pluribus.
Author, et al. Expires 13 January 2019 [Page 7]
Internet-Draft Xml2rfc Vocabulary V3 Elements July 2018
Ordered list, third bullet, second element:
+-------------------------------------------+
| |
| Artwork |
| |
+-------------------------------------------+
Ordered list, third bullet, third element: Definition list,
element dt.
Element dd. Nisi mihi Phaedrum, inquam, tu mentitum aut
Zenonem misisti.
Ordered list, third bullet, fourth element:
Figure
Figure
Figure
Figure 2: Some figure name
1. Ordered list, third bullet, fifth element: Ordered list,
element li. Text only.
2. Ordered list, third bullet, fifth element: Ordered list,
element t. Nam cum ad me de virtute misisti. Quae est enim
contra Cyrenaicos satis acute, nihil ad iucunde vivendum
reperiri posse, quod coniunctione tali sit aptius. Quae etsi
mihi nullo modo nec divelli nec distrahi possint, sic de
iustitia iudicandum est, quae non modo non impediri rationem
amicitiae, si summum bonum diceret, primum in eo essent.
# Ordered list, third bullet, sixth element
A = 1;
* Ordered list, third bullet, seventh element: Unordered list,
element li. Text only.
* Element t. Atque ut odia, invidiae, despicationes adversantur
voluptatibus, sic amicitiae non posse iucunde vivi, nisi
sapienter, honeste iusteque vivatur, nec sapienter, honeste,
iuste, nisi iucunde. Democritea dicit perpauca mutans, sed
ita, ut ea, quae audiebamus, conferebamus, neque erat umquam
controversia, quid ego intellegerem, sed quid probarem. See
Appendix A, Section 3.1 of
[I-D.levkowetz-xml2rfc-v3-implementation-notes] and also
Appendix A.1 of Key Words Case Ambiguity [RFC8174] / Key Words
Case Ambiguity [RFC8174], Appendix A.1 / Key Words Case
Ambiguity [RFC8174] (Appendix A.1).
Author, et al. Expires 13 January 2019 [Page 8]
Internet-Draft Xml2rfc Vocabulary V3 Elements July 2018
4. 4.1 Sublist with '%p' parent counter element in the list
counter, item one
4.2 Item two.
4.3 Item three.
and consider what would happen if there were any text between the <li> and <ol> at

xml2rfc/tests/input/elements.xml

Lines 472 to 473 in fdb6fb8

<li>
<ol type="%p%d">

@kesara kesara marked this pull request as draft July 10, 2024 00:56
@kesara
Copy link
Member Author

kesara commented Jul 10, 2024 •
edited
Loading

I have converted PR to a draft.
The changes from e889f93 add a new line when with the list style to both ol and ul.
This was skipped previously.
Specifically, this adds list style + new line when li contains an artwork, figure, or sourcecode as the first child.
See how this changed test outputs in ca8286a
Note that the test output has date changes and multiple content changes propagated because of the addition of new lines.

e889f93 fixes #1138

@kesara kesara changed the title fix: Render ol items correctly fix(text): Render ol items correctly Jul 10, 2024
@kesara kesara changed the title fix(text): Render ol items correctly fix(text): Render list items correctly Sep 12, 2024
@kesara kesara added the rpat Issues needing attention from RFC Production Advisory Team label Sep 12, 2024
@kesara kesara marked this pull request as ready for review December 9, 2024 00:25
@kesara kesara requested a review from rjsparks December 9, 2024 00:25
@kesara
Copy link
Member Author

kesara commented Dec 17, 2024 •
edited
Loading

@rjsparks
Here's diffs for RFCs that has <ul> or <ol> as first element inside a <li>1.
When empty="true" is used, this change will not affect the output of that RFC.

Footnotes

  1. https://t4.fq.nz/xml2rfc-1137/output.txt ↩

@rjsparks
Copy link
Member

The above list is a bit confusing? I only see impact from this PRs change on rfc9599.

@kesara
Copy link
Member Author

kesara commented Dec 18, 2024

The above list is a bit confusing? I only see impact from this PRs change on rfc9599.

In most case there's empty="true" attribute set on the element or the parent level, hence no changes.

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

@cabo cabo cabo left review comments

@rjsparks rjsparks Awaiting requested review from rjsparks

Assignees
No one assigned
Labels
rpat Issues needing attention from RFC Production Advisory Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Missing style in <ul> missing number in text output of <ol>
3 participants
@kesara @rjsparks @cabo