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

Remove occurences from the path #302

Merged
merged 8 commits into from
Dec 1, 2019
Merged

Remove occurences from the path #302

merged 8 commits into from
Dec 1, 2019

Conversation

robUx4
Copy link
Contributor

@robUx4 robUx4 commented Nov 3, 2019

Ref. #292

The Matroska Schema will need to be updated when this is merged.

@robUx4 robUx4 added enhancement Add more content to the document, explaning more things clarifications Improve the readability and comprehension of the specs XML Schema EBML Schema validation labels Nov 3, 2019
robUx4 added a commit that referenced this pull request Nov 3, 2019
@robUx4
Copy link
Contributor Author

robUx4 commented Nov 17, 2019

I'd like to go ahead with this because it's blocking my "code generators" from the Matroska EBML Schema.

IIRC the goal was originally that with just a path you can tell how to interpret the element. But that's not the case, there's not the type for example. In the end the Schema is more complete.

The path is still important to describe new elements in "foreign" documents without ambiguity.

An integer expressing the minimum permitted number of occurrences of this EBML Element within its Parent Element. The minOccurs value MUST be equal to the EBMLMinOccurrence value of the path.
The minOccurs is an integer representing the minimum permitted number of occurrences of this EBML Element within its Parent Element.

Each instance of the Parent Element MUST contain at least this many instances of this EBML Element.
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps 'contain' is the wrong word. As minOccurs=1 with a default value does not need to be 'contained' within the parent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the same wording that was used in EBMLMinOccurrence, dating back to 40fe4cb.

In the next sentence(s) it actually explains in which case the element can be omitted: EBML Elements with minOccurs set to "1" that also have a default value declared are not REQUIRED to be stored.
It was all on the same line before and still in the same paragraph.

For maxOccurs there's no such thing. How about:
Each instance of the Parent Element MUST contain at most this many instances of this EBML Element, including the unwritten mandatory element with a default value, see (#note-on-the-use-of-default-attributes-to-define-mandatory-ebml-elements).

specification.markdown Outdated Show resolved Hide resolved
Copy link
Contributor

@dericed dericed left a comment

Choose a reason for hiding this comment

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

minor comments, but this is a nice improvement.

@dericed
Copy link
Contributor

dericed commented Nov 21, 2019

After merging #303 this needs a rebase.

@dericed
Copy link
Contributor

dericed commented Nov 27, 2019

Any further comments? I suggest merging soon.

@robUx4
Copy link
Contributor Author

robUx4 commented Nov 28, 2019

If the maxOccurs change is OK with you, you can merge.

@dericed dericed merged commit b33cd09 into master Dec 1, 2019
@dericed dericed deleted the del-path-occurences branch December 1, 2019 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarifications Improve the readability and comprehension of the specs enhancement Add more content to the document, explaning more things XML Schema EBML Schema validation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants