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

accordion-middle should support zero dots #134

Closed
webern opened this issue Aug 9, 2016 · 7 comments
Closed

accordion-middle should support zero dots #134

webern opened this issue Aug 9, 2016 · 7 comments
Milestone

Comments

@webern
Copy link
Contributor

webern commented Aug 9, 2016

This is what the specification says

<xs:simpleType name="accordion-middle"> <xs:annotation> <xs:documentation>The accordion-middle type may have values of 1, 2, or 3, corresponding to having 1 to 3 dots in the middle section of the accordion registration symbol.</xs:documentation> </xs:annotation> <xs:restriction base="xs:positiveInteger"> <xs:minInclusive value="1"/> <xs:maxInclusive value="3"/> </xs:restriction> </xs:simpleType>

This is what I think it should say

<xs:simpleType name="accordion-middle"> <xs:annotation> <xs:documentation>The accordion-middle type may have values of 1, 2, or 3, corresponding to having 1 to 3 dots in the middle section of the accordion registration symbol.</xs:documentation> </xs:annotation> <xs:restriction base="xs:nonNegativeInteger"> <xs:minInclusive value="0"/> <xs:maxInclusive value="3"/> </xs:restriction> </xs:simpleType>

Evidence http://www.duckmandu.com/notation/
Shows accordion symbols with zero dots in the middle.

Disclaimer: I don't know anything about Accordion.

@jsawruk
Copy link
Contributor

jsawruk commented Aug 9, 2016

If you're going to change the range of values from 0 to 3, then the documentation should reflect this as well. Here's my proposal based on yours:

<xs:simpleType name="accordion-middle"> <xs:annotation> <xs:documentation>The accordion-middle type may have values of 0, 1, 2, or 3, corresponding to having 0 to 3 dots in the middle section of the accordion registration symbol.</xs:documentation> </xs:annotation> <xs:restriction base="xs:nonNegativeInteger"> <xs:minInclusive value="0"/> <xs:maxInclusive value="3"/> </xs:restriction> </xs:simpleType>

@webern
Copy link
Contributor Author

webern commented Aug 9, 2016

Right, thanks for catching that.

@mdgood
Copy link

mdgood commented Sep 21, 2016

Thanks for raising this issue. 0 dots in a section are represented in MusicXML 3.0 by omitting the corresponding element. I think that the issue here is clarifying the documentation for the accordion-registration element and its children, so I will add a Documentation label here.

@mdgood
Copy link

mdgood commented Oct 4, 2016

This could be clarified by adding "This element is omitted if no dots are present" to the end of the accordion-middle documentation. The sentence "This element is omitted if no dot is present" could likewise be added to the documentation for the accordion-high and accordion-low elements.

@mdgood
Copy link

mdgood commented Jan 3, 2017

Pull request #162 addresses this issue. The documentation updates turned out a little bit different than what I proposed earlier. @webern, could you please review the pull request to be sure the documentation is clarified appropriately?

The copyright notices were also updated for 2017. That change is in a separate commit from the documentation changes for this issue.

@webern
Copy link
Contributor Author

webern commented Jan 3, 2017

Yes, the rewrite is more clear. Thank you. There is a wrong word typo
If not dots are present
should be
If no dots are present

@mdgood
Copy link

mdgood commented Jan 3, 2017

Thanks for the quick review. The typo should be fixed now.

@mdgood mdgood removed the In Progress label Jan 4, 2017
@mdgood mdgood closed this as completed Jan 4, 2017
@mdgood mdgood added this to the V3.1 milestone Jan 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants