Hi,
On 2014/09/02 13:45:45, dak wrote:
So I went through a review after all and found a few niggles that seem
worth
fixing. Probably does not require a full countdown and stuff, but
another
patchy run might be warranted in order to avoid typos.
Thanks for reviewing, and for giving me green light for pushing without
next full countdown (i take this as a sign of trust). However,
considering how my recent patches tended to have problems, i'll leave it
for a full cycle, especially that i'm not in hurry with this one.
https://codereview.appspot.com/137760043/diff/1/lily/engraver-group.cc
File lily/engraver-group.cc (right):
https://codereview.appspot.com/137760043/diff/1/lily/engraver-group.cc#newcode99
lily/engraver-group.cc:99: SCM meta = info.grob ()->get_property
("meta");
On 2014/09/02 13:33:02, dak wrote:
I'm not sure whether meta lookups would be wanted in the statistics
when we have
the stat-collection enabled.
Hmm. I don't know either. I'd really like to get rid of
internal_get_property where possible (so that no one will be confused as
i was), but maybe here it'd be better to err on the safe side...
https://codereview.appspot.com/137760043/diff/20001/lily/grob.cc
File lily/grob.cc (right):
https://codereview.appspot.com/137760043/diff/20001/lily/grob.cc#newcode470
lily/grob.cc:470: SCM min_ext = get_property (min_ext_name);
On 2014/09/02 13:45:44, dak wrote:
This one is bad since it breaks memoization. Either leave it alone or
do
(properly formatted)
SCM min_ext = a == X_AXIS ? get_property ("minimum-X-extent") :
get_property
("minimum-Y-extent")
Done.
https://codereview.appspot.com/137760043/diff/20001/lily/self-alignment-interface.cc
File lily/self-alignment-interface.cc (right):
https://codereview.appspot.com/137760043/diff/20001/lily/self-alignment-interface.cc#newcode54
lily/self-alignment-interface.cc:54: SCM align (me->get_property (sym));
On 2014/09/02 13:45:45, dak wrote:
See above.
Done.
https://codereview.appspot.com/137760043/diff/20001/lily/self-alignment-interface.cc#newcode125
lily/self-alignment-interface.cc:125: ? me->get_property
("self-alignment-X")
On 2014/09/02 13:45:45, dak wrote:
Here you do it properly!
Acknowledged.
https://codereview.appspot.com/137760043/diff/20001/lily/span-bar-engraver.cc
File lily/span-bar-engraver.cc (right):
https://codereview.appspot.com/137760043/diff/20001/lily/span-bar-engraver.cc#newcode85
lily/span-bar-engraver.cc:85: SCM vis = bars_[0]->get_property
("break-visibility");
On 2014/09/02 13:45:45, dak wrote:
This causes the symbol to be memoized three times in a row. However,
only the
first time the code is being run, so I'd say never mind. The
readability should
be worth it.
Acknowledged.
https://codereview.appspot.com/137760043/
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
