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.

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

Add interleave element (and deprecate sequence/@preserveOrder) #2538

Open
wants to merge 11 commits into
base: dev
Choose a base branch
from

Conversation

joeytakeda
Copy link
Contributor

Resolves #2154 by adding the interleave element and deprecating sequence/@preserveOrder

At the moment, I'm leaving this as draft as per @hcayless' observation that we should figure out how best to validate the defined content model to prevent overlap. IMO, we should probably warn about this in the prose and in the remarks for the element, but true validation will probably need to be left up to the ODD processor.

* New elementSpec and updated TD
* Still (possibly) need to address hcayless' point about validity of the construct (i.e. no overlap)
@joeytakeda joeytakeda marked this pull request as ready for review April 13, 2024 16:24
@joeytakeda joeytakeda requested a review from HelenaSabel April 13, 2024 16:25
@joeytakeda
Copy link
Contributor Author

Moving this to be ready for review, since I think the question of validity is probably something that needs to be handled by the processor (but have assigned mostly ATOP folks for review to see what they think about that)

@sydb
Copy link
Member

sydb commented May 17, 2024

Just to make the concern initially raised by @hcayless (on the ticket) echoed by @joeytakeda (here) explicit …

In RELAX NG it is not legal to have the same element occur in an interleave more than once. And because interleave “reaches in” to the patterns listed, this is hard to check for (other than by running a RELAX NG processor like jing … I just discovered that neither trang nor nxml-mode object).

So, for example, the following is not allowed in RELAX NG:

start = element blood { ( a & b & o & ab ) }

a = element a { empty }
b = element b { empty }
o = element o { empty }
ab = ( a | b )

And it does not matter whether the ab pattern is defined as an alternation (<rng:choice> or |), a sequence (<rng:group> or ,), or an interleave (<rng:interleave> or &). In all cases the reference to the pattern ab throws an error. (The message is “the element "b" can occur in more than one operand of "interleave"”, which I, for one, find confusing.)

Copy link
Member

@sydb sydb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer the issues I note be addressed before merge, but it is not a big deal. (And I may get to the easier ones myself, soon.)
That said, we do need a discussion on the fact that use of <interleave> in an ODD means that the RELAX NG generated by either the Stylesheets or by ATOP will not be transformable into DTD language.

cardinality specified. Only one of the references listed as
children of <gi>alternate</gi> may appear, although the
cardinality specified whereas references listed as children
of <gi>interleave</gi> may appear in any order. Only one of the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can-of-worms department.

(Note to @joeytakeda — most of the problem here is not from your improvement, but has been here all along and is just foregrounded by your change.)

This sentence does not mention that cardinality of the stuff inside <interleave>. But on thinking of how to re-word it, I realize that these sentences discuss the cardinality of references, when what we are talking about is the cardinality of the thing referenced. I am not sure what, if anything, we should do about this.

P5/Source/Guidelines/en/TD-DocumentationElements.xml Outdated Show resolved Hide resolved
P5/Source/Specs/interleave.xml Outdated Show resolved Hide resolved
@@ -19,7 +19,8 @@
</constraint>
</constraintSpec>
<attList>
<attDef ident="preserveOrder">
<attDef ident="preserveOrder" validUntil="2025-03-15">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that a) <interleave> did not make it into the recent release, and b) use of @preserveOrder still works in the Stylesheets, I think the @validUntil date should be moved up to late 2025 or early 2026.

sydb added 5 commits July 19, 2024 21:26
Although the whole point of this branch is to allow use of the inerleave
element in content models, because you cannot generate a DTD from an ODD
that uses interleave, we do not want to use it in the source of the
Guidelines themselves. (Until we drop support for DTDs.)
@tuurma
Copy link
Contributor

tuurma commented Nov 13, 2024

F2F discussion in Buenos Aires: We should provide a <remark> in the <interleave> element spec to caution people working with elements belonging to multiple classes, or when using classRef.

@trishaoconnor trishaoconnor added the Status: Pending pending action described in a comment, to return to discussion before further action will be taken label Nov 27, 2024
@trishaoconnor trishaoconnor added this to the Guidelines 4.9.0 milestone Nov 27, 2024
@raffazizzi
Copy link
Contributor

Subgroup @sydb @joeytakeda @raffazizzi suggest also creating a Stylesheet ticket to check validity of rng before continuing build in order to catch validation errors caused by interleave/classRef (and maybe more!)

@joeytakeda
Copy link
Contributor Author

Stylesheets ticket created here: TEIC/Stylesheets#719 — for discussion at the next council meeting is whether the approval of this PR (e.g. the additional of interleave) is contingent on the validation process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Pending pending action described in a comment, to return to discussion before further action will be taken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create an interleave element
6 participants