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 plica element & fix stem element #722

Merged
merged 8 commits into from Oct 29, 2020

Conversation

martha-thomae
Copy link
Contributor

@martha-thomae martha-thomae commented Oct 22, 2020

This PR adds support for a <plica> element to be used as child of <note>. The plica element has two attributes (which are shared with the already existing <stem> element): @dir and @len. A <note> element can have only one child <plica>.

This PR also contains some corrections regarding the <stem> element and its attributes:

  • Removed the line <memberOf key="model.noteModifierLike"/> from the <stem> element classes since, in addition to allow <stem> as a child of <note> (which was the intention), it also allowed <stem> to be a direct child of <layer>. Instead, I added <stem> directly into the content of <note>.
  • Substitute the @length attribute of <stem> by @len (to be more consistent with MEI).
  • Add @color attribute.

For both stem and plica, added corresponding anl, log, ges, and vis attribute classes.

Acknowledgement:
Thanks to @pe-ro for helping me getting this PR ready. He noticed issues with it that are fixed now.

This was done by removing the 'model.noteModifierLike' reference from the classes of the 'plica' element (which allowed plica to be both a child of note and layer) and adding it to the content model of the 'note' element instead.
Other changes included in this commit are:
- Add attribute classes for logical-, analytical-, gestural-, and visual-domain attributes of plica (rather than just one general 'att.plica' class).
- Move the attributes 'dir' and 'len' into the 'att.plica.vis' class for visual-domain plica-related attributes.
- Within the plica element, add references to 'att.plica.log', 'att.plica.vis', 'att.plica.anl', and 'att.plica.ges' plica-related attribute classes, and to the 'att.facsimile' class (for the 'facs' attribute).
- Change the 'att.STEMPROPERTIES.mensural' class name to 'att.stem.vis', and add empty classes for the other three domains (att.stem.log, att.stem.ges, and att.stem.anl). Then, copy the definitions of 'att.stem.vis' into 'att.plica.vis' for the attributes related to stem direction and length. This implied that the name of the plica attribute for encoding the length of its stem was changed to 'length' (rather than 'len').
- Change the value of 'length' in the stem element to encode only positive numbers (since there is already a 'dir' attribute for indicating direction).
- Modify the stem element in a similar way as we did for 'plica' in the previous commit: remove the 'model.noteModifierLike' from its classes (this was causing the stem to be a valid child of layer) and instead add stem to the content model of note, add attribute classes for the four domains, and add 'att.facsimile'.
- The 'len' attribute is used in all other cases, so 'att.plica.vis' and 'att.stem.vis' are using this attribute name too.
- Change back the data type of this 'len' attribute to data.MEASUREMENTABS.
Copy link
Member

@kepper kepper left a comment

Choose a reason for hiding this comment

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

We should trust in the work of our IGs, and @pe-ro has already had a look from a technical perspective – ready to go… 

@kepper kepper added the Status: Ready To Merge indicates that a pull request is ready for merging label Oct 29, 2020
@kepper kepper merged commit 0bd765b into music-encoding:develop Oct 29, 2020
1 check was pending
@kepper
Copy link
Member

kepper commented Oct 29, 2020

thanks @martha-thomae 👍

@musicEnfanthen musicEnfanthen removed the Status: Ready To Merge indicates that a pull request is ready for merging label Jan 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants