#4937 [GSoC] Implement cross-voice dynamic spanners

[Nathan Chou]

[GSoC] Implement cross-voice dynamic spanners

\= was changed to allow setting spanner-share-context property on
events. spanner-id was consequently changed to a key.

Spanner_engraver class modifies a new context property sharedSpanners.
Spanner information, including the Spanner object and the current
voice the spanner belongs to, is stored in this property. If a context
above Voice is used, other voices may see the spanner and modify it.

Dynamic_engraver and Dynamic_align_engraver were changed to support
cross-voice spanners using the above mechanics. I rewrote portions of
Dynamic_align_engraver to do this more easily.

Examples:
<< { c\=Score.hello\< d e f } \ { e f g\=Score.hello! a } >>
\new Staff { << { c d e\=Staff.hello\< f } >> << { g> f!\=Staff.hello! e d } >> }
\new Staff { c d e\=1\< f\=2\< g f\=1! e\=2! d }

http://codereview.appspot.com/304160043

[Anonymous]

• Description has changed:

Diff:

• Needs: -->
• Patch: new --> review

[Anonymous]

Passes make, make check and a full make doc.

I don't see any reg tests generated (apart from the default one) nor any reg tests created for this issue (in case we need to have a reg test).

[Nathan Chou]

Thanks, I will add some reg tests

[Nathan Chou]

Made some changes to Spanner_engraver, and added reg tests

http://codereview.appspot.com/304160043

[Anonymous]

Passes make, make check and a full make doc.

[Anonymous]

Patch on countdown for July 27th.

Nathan, just out of curiosity, does this re-write mean we need to add any new information (for users) in the Notation Reference anywhere or perhaps add something to changes.tely?

If so, there is no need for you to do that in this patch; we can make a new tracker for these (so they are not forgotten). Again, no need for you to necessarily write the doc if there is anything new to add, but some notes for those doc writers who (like me) may not know what all these new changes mean from a user's point of view, would enable them to make a patch instead.

[Nathan Chou]

Yes: essentially the \= command was changed to allow writing cross-voice spanners. It can now take a key-list of two elements indicating the spanner's id and context to be shared in. I can help with writing new documentation when the implementation is settled.

Per the code review, I need to make significant changes to this patch; could you remove it from this countdown? Thanks

[David Kastrup]

Nathan, I am sorry for the late feedback (I tend to procrastinate far too much). Sometimes one just needs to see the results from one approach to figure out how it could/should/might be done better and just how the impact of a particular design is.

Now the change to \= is mostly independent of the bulk of the code, particularly the change of spanner-id from string to key?. Following through with that will, in the long haul, require convert-ly rules, changed documentation, change logs and so on. I assume/hope that your work on this aspect is mostly contained in separate commits (if you need support with using Git for factoring work well, just holler), so separating those changes out should be reasonably easy.

I don't think that it makes sense for you to get distracted on getting that piece of history straightened out (this cleanup work is not really a seminal part of your project and having reviews and cosmetic changes go back and forth on that would take valuable time and energy from your project), so I'll offer to iron that out in a separate issue. If you can push your current work to a branch (do you have push privileges?) or send them to me formatted via git-email, I can make sure that your work will rebase cleanly or at least easily on the separately committed changes in order to keep the hassle to you to a minimum. If necessary, I can do any rebase/merge necessary on your work branch for you so that you don't get disrupted with it.

[Nathan Chou]

No problem and thanks! I did somewhat organize the changes into commits, but I don't have push privileges, so I will email you them.

[Anonymous]

I assume from the comments above that this tracker is going to have multiple 'countdowns' (so to speak)?

I'd suggested, to keep Nathan on track, that we have a separate tracker for changes+doc so that we can 'worry' about that later on.

I'll set this to 'push' and let David do whatever needs to be done with this tracker (e.g. set back to needs_work or fixed as appropriate).

[David Kastrup]

As I understood the discussion and reply, Nathan intended to try factoring/abstracting the multiple spanner functionality in a different manner. That would make it a bad idea to push now. I offered to take on a fairly trivial subset on my own (which may have repercussions and followup work not really related to the multiple spanner work) and make sure that this does not interfere with Nathan's work. So for the current patch status of this issue it would mean that it's "needs_work".

If I understood anything wrong, holler.

[Nathan Chou]

Preliminary refactor to use multiple engraver instances to handle multiple spanners

http://codereview.appspot.com/304160043

[Nathan Chou]

Fix issue with unterminated warnings; add reg tests.

http://codereview.appspot.com/304160043

[Anonymous]

Passes make, make check and a full make doc.

[Anonymous]

From the comments on Rietveld this is not yet ready, so I will leave this on Review and hopefully someone can help Nathan with his questions.

[Nathan Chou]

Fixed (though not sure if with best approach) issue with filtered acknowledgers. May try coding alternative approach to see how it compares.

http://codereview.appspot.com/304160043

[Anonymous]

Passes make, make check and a full make doc.

[Anonymous]

Patch on countdown for August 30th