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

BREAKING(modules): Change macro name of data.OTHERSTAFF #932

Merged

Conversation

rettinghaus
Copy link
Member

data.OTHERSTAFF is used for @beam.with and @stem.with. The latter one is used to combine voices in part writing, i.e., merge stems from different layers into one. So the description of this macro ("on another staff") is not correct.
But even for @beam.with it is not very precise, because this could happen within one staff, or point to a staff that has multiple layers.

This PR proposes to change the name from data.OTHERSTAFF to data.NEIGHBORINGLAYER, because it actually points to the next layer above or below the current one (even if this is "on another staff"). Descriptions are updated accordingly.

This will not(!) change the values or anything else.

@github-actions github-actions bot added the Component: Core Schema changes to source/modules/* (assigned automatically) label Mar 15, 2022
@bwbohl bwbohl added Backwards-Compatibility: breaking discussed or proposed changes would result in a backward incompatibility / require a major release Status: Needs Discussion and removed Status: Needs Review labels Apr 19, 2022
@bwbohl
Copy link
Member

bwbohl commented Apr 19, 2022

Thanks for bringing this up @rettinghaus! I think being precise in the naming of things is always a good idea. But I somehow see a much bigger problem in the imprecision of the values above and below, in both cases staff and layer): what are they referring to, graphical position? logical position in the XML? What if there are multiple layers on the referred staff? What if the notes to be stemmed with are on the the above staff but not on the lowest (graphical? or logical?) layer – after all I think there is no definite rule on how to stack layers in the XML according to their graphical position, or is there?

image

@bwbohl
Copy link
Member

bwbohl commented Apr 21, 2022

So the question concerning your proposed changes would be: what are ”adjacent layers”?

source/modules/MEI.xml Outdated Show resolved Hide resolved
@ahankinson
Copy link
Member

Could you provide an example of where you might use "beam with" or "stem with" on neighbouring layers on the same staff?

kepper
kepper previously approved these changes Oct 1, 2022
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.

After discussion at 2022 Berlin dev meeting, @musicEnfanthen, @rettinghaus, @fujinaga, @pe-ro and @kepper agree to accept this.

@kepper
Copy link
Member

kepper commented Oct 1, 2022

The current proposal doesn't easily support connecting layers 1 and 3 plus 2 and 4 of four instruments spread across two staves. However, we didn't do that in the past, so we can skip it once more ;-)

@kepper
Copy link
Member

kepper commented Oct 1, 2022

IMG_6208

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.

still good, now with languages

@musicEnfanthen musicEnfanthen merged commit 05d88d6 into music-encoding:develop Oct 1, 2022
@musicEnfanthen
Copy link
Member

Thanks everyone! Merged at MEI Developer meeting 2022

@rettinghaus rettinghaus deleted the develop-otherlayer branch October 1, 2022 15:19
@musicEnfanthen musicEnfanthen changed the title Change macro name of data.OTHERSTAFF BREAKING(modules): Change macro name of data.OTHERSTAFF Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backwards-Compatibility: breaking discussed or proposed changes would result in a backward incompatibility / require a major release Component: Core Schema changes to source/modules/* (assigned automatically)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants