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
