http://codereview.appspot.com/4213042/diff/34032/input/regression/footnotes.ly File input/regression/footnotes.ly (right):
http://codereview.appspot.com/4213042/diff/34032/input/regression/footnotes.ly#newcode7 input/regression/footnotes.ly:7: \book { } Sorry, I meant wrap all the music inside a \book { }, otherwise the regression test won't show any footnotes at the bottom of each page. http://codereview.appspot.com/4213042/diff/34032/lily/balloon.cc File lily/balloon.cc (right): http://codereview.appspot.com/4213042/diff/34032/lily/balloon.cc#newcode90 lily/balloon.cc:90: // ugh...code dup...hopefully can be consolidated w/ above one day Can't you move most of the stencil creation to a separate method? http://codereview.appspot.com/4213042/diff/34032/lily/balloon.cc#newcode99 lily/balloon.cc:99: if (parent->broken_intos_.size () == 0) if (!parent->is_broken ()) http://codereview.appspot.com/4213042/diff/34032/lily/dynamic-align-engraver.cc File lily/dynamic-align-engraver.cc (right): http://codereview.appspot.com/4213042/diff/34032/lily/dynamic-align-engraver.cc#newcode92 lily/dynamic-align-engraver.cc:92: * robust_scm2double (info.grob ()->get_property ("Y-offset"), 0.0) > 0.0)) { This hard-codes an exception to the footnote's extent; I think it would be better to add it in every case then rely on overriding Y-extent if it shouldn't take up space. This is still a kludge though, since there are other alignment spanners which contain grobs we might like to add footnotes to (e.g., pedals). Ideally, you need a more flexible mechanism which can be applied to any relevant axis group. http://codereview.appspot.com/4213042/diff/34032/lily/footnote-engraver.cc File lily/footnote-engraver.cc (right): http://codereview.appspot.com/4213042/diff/34032/lily/footnote-engraver.cc#newcode4 lily/footnote-engraver.cc:4: Copyright (C) 2006--2011 Han-Wen Nienhuys <[email protected]> Fix copyright. http://codereview.appspot.com/4213042/diff/34032/lily/footnote-engraver.cc#newcode76 lily/footnote-engraver.cc:76: b->set_property ("parent-spanner", s->self_scm ()); What happens to the Y-parent setting above? If it's valid, you shouldn't need an object pointer. If you do need it, it should be set_object () (and for reading, get_object ()) http://codereview.appspot.com/4213042/diff/34032/lily/footnote-engraver.cc#newcode98 lily/footnote-engraver.cc:98: { remove { } http://codereview.appspot.com/4213042/diff/34032/lily/footnote-engraver.cc#newcode119 lily/footnote-engraver.cc:119: "Footnote ", "Footnote " "FootnoteSpanner ", Perhaps rename Footnote FootnoteItem. http://codereview.appspot.com/4213042/diff/34032/lily/footnote-engraver.cc#newcode122 lily/footnote-engraver.cc:122: "", "currentCommandColumn " "currentMusicalColumn " "whichBar ", http://codereview.appspot.com/4213042/diff/34032/lily/page-layout-problem.cc File lily/page-layout-problem.cc (right): http://codereview.appspot.com/4213042/diff/34032/lily/page-layout-problem.cc#newcode77 lily/page-layout-problem.cc:77: paper->self_scm ()); indent http://codereview.appspot.com/4213042/diff/34032/lily/page-layout-problem.cc#newcode104 lily/page-layout-problem.cc:104: if (stencil->extent (Y_AXIS).length() > 0.0) length () http://codereview.appspot.com/4213042/diff/34032/lily/system.cc File lily/system.cc (right): http://codereview.appspot.com/4213042/diff/34032/lily/system.cc#newcode236 lily/system.cc:236: if ((all_elts[i]->name () == "Footnote") || (all_elts[i]->name () == "FootnoteSpanner")) cleaner with all_elts[i]->internal_has_interface (ly_symbol2scm ("footnote-interface")) assuming you add footnote-interface to FootnoteSpanner http://codereview.appspot.com/4213042/diff/34032/lily/system.cc#newcode247 lily/system.cc:247: if (footnote_grobs_.size () == 0) !footnote_grobs_size () http://codereview.appspot.com/4213042/diff/34032/ly/engraver-init.ly File ly/engraver-init.ly (right): http://codereview.appspot.com/4213042/diff/34032/ly/engraver-init.ly#newcode537 ly/engraver-init.ly:537: \consists "Footnote_engraver" move back to Voice (sorry! :) http://codereview.appspot.com/4213042/diff/34032/scm/define-grob-interfaces.scm File scm/define-grob-interfaces.scm (right): http://codereview.appspot.com/4213042/diff/34032/scm/define-grob-interfaces.scm#newcode92 scm/define-grob-interfaces.scm:92: '(footnote-text)) + parent-spanner if still required http://codereview.appspot.com/4213042/diff/34032/scm/define-grob-properties.scm File scm/define-grob-properties.scm (right): http://codereview.appspot.com/4213042/diff/34032/scm/define-grob-properties.scm#newcode298 scm/define-grob-properties.scm:298: (footnote-text ,markup? "A footnote for the grob.") + parent-spanner if required (add to `all-internal-grob-properties') http://codereview.appspot.com/4213042/diff/34032/scm/define-grobs.scm File scm/define-grobs.scm (right): http://codereview.appspot.com/4213042/diff/34032/scm/define-grobs.scm#newcode180 scm/define-grobs.scm:180: (annotation-balloon . #t) mixing space with tabs (also Footnote and FootnoteSpanner) http://codereview.appspot.com/4213042/diff/34032/scm/define-grobs.scm#newcode886 scm/define-grobs.scm:886: (footnote-text . #f) remove http://codereview.appspot.com/4213042/diff/34032/scm/define-markup-commands.scm File scm/define-markup-commands.scm (right): http://codereview.appspot.com/4213042/diff/34032/scm/define-markup-commands.scm#newcode142 scm/define-markup-commands.scm:142: (define-markup-command (draw-hline layout props) still needs simplifying via draw-line http://codereview.appspot.com/4213042/ _______________________________________________ lilypond-devel mailing list [email protected] http://lists.gnu.org/mailman/listinfo/lilypond-devel
