Most of my comments below resemble each other, but up here I want to suggest that if you go down the road of generalizing the arpeggio grob, the places you'd have to do work are in the rhythmic column engraver and in the several NoteColumn functions that look for an arpeggio. Instead, this would be generalized to LeftSideIndication and maybe a LeftSideIndicationSpanner to coordinate the horizontal deployment.
Great work in making your way through a lot of Lily code to get this up and running! I get the sense that you have a good understanding of the code base, which will serve you well for future endeavors. Cheers, MS http://codereview.appspot.com/4807053/diff/18001/lily/brace-engraver.cc File lily/brace-engraver.cc (right): http://codereview.appspot.com/4807053/diff/18001/lily/brace-engraver.cc#newcode1 lily/brace-engraver.cc:1: /* Check out my comment for Span_brace_engraver. Of all the engravers in LilyPond, I think that the new-fingering-engraver is the one that can act as the best model for the creation of new engravers insofar as it handles many different grobs with the same method. I think that this can be rolled into the arpeggio engraver with a few extra functions. http://codereview.appspot.com/4807053/diff/18001/lily/span-brace-engraver.cc File lily/span-brace-engraver.cc (right): http://codereview.appspot.com/4807053/diff/18001/lily/span-brace-engraver.cc#newcode1 lily/span-brace-engraver.cc:1: /* Is there any way to combine this with the span arpeggio engraver? It seems like stop_translation_timestep could be moved to another function (ie process_spanning_grob) and then could handle multiple grobs (span_brace_, span_arpeggio_, etc.). http://codereview.appspot.com/4807053/diff/18001/lily/span-brace-engraver.cc#newcode6 lily/span-brace-engraver.cc:6: I don't really remember having anything to do with this file...was I involved in this? I'm getting old if I can't remember this sorta thing! If you're comfortable adding your e-mail address, that helps people who want to edit the file later. http://codereview.appspot.com/4807053/diff/18001/scm/define-grobs.scm File scm/define-grobs.scm (right): http://codereview.appspot.com/4807053/diff/18001/scm/define-grobs.scm#newcode501 scm/define-grobs.scm:501: Just food for thought: currently, the "Script" grob encompasses many things (staccati, tenuti, etc.), all of which could be their own grobs, but are common and multiple enough to be grouped under one grob. In general, I would work with the rule of "one, two, and many." That is, if you see something being one grob, then leave it as one. If there are two (or max(ish) three) that are similar (like the piano pedal stuff) it is ok to have multiple grobs. But once you cross the threshold into many, it's better to handle them like scripts are handled. I agree that braces are currently too-coupled with arpeggio's functionality, but before they become a separate grob, I'd encourage you to think about how plausible it is that other grobs like this one will need to be created. If you can envision many, then roll this into arpeggio (or even change the name of arpeggio to SideSpanner to be more neutral about the actual content), allow for multiple ones attached to one note column, have some sorta interface/uber-grob for determining horizontal order, and go down that route. http://codereview.appspot.com/4807053/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
