Re: Various clean-ups in stems and beams. (issue 6584045)

[dak]

http://codereview.appspot.com/6584045/diff/1/lily/beam.cc
File lily/beam.cc (right):

http://codereview.appspot.com/6584045/diff/1/lily/beam.cc#newcode197
lily/beam.cc:197: Grob *me = unsmob_grob (smob);
Looking at the combination of this and is_kievan, it would appear that
the expected response when calling Beam::calc-is-kievan (why no question
mark in the name?) with a non-Grob is a segmentation fault.

That's sub-fabulous.

http://codereview.appspot.com/6584045/diff/1/lily/stem.cc
File lily/stem.cc (right):

http://codereview.appspot.com/6584045/diff/1/lily/stem.cc#newcode815
lily/stem.cc:815: Grob *beam = unsmob_grob (me->get_object ("beam"));
Again: it would appear that calling this with a non-grob is intended to
crash LilyPond.  Is there a particular reason you write your callbacks
without typechecking their arguments?

We have things like LY_ASSERT_TYPE for a reason.

http://codereview.appspot.com/6584045/diff/1/lily/stem.cc#newcode833
lily/stem.cc:833: Grob *beam = unsmob_grob (me->get_object ("beam"));
See above.

http://codereview.appspot.com/6584045/diff/1/scm/output-lib.scm
File scm/output-lib.scm (right):

http://codereview.appspot.com/6584045/diff/1/scm/output-lib.scm#newcode70
scm/output-lib.scm:70: (next-stem (cadr stems-grobs))
The above line may set stems-grobs to '(), in which case this line will
bomb out with a Scheme error.  Where is the point?

http://codereview.appspot.com/6584045/diff/1/scm/output-lib.scm#newcode85
scm/output-lib.scm:85: (left-height (if (= direction DOWN)
Is direction sure to be non-null?

http://codereview.appspot.com/6584045/diff/12001/lily/stem.cc
File lily/stem.cc (right):

http://codereview.appspot.com/6584045/diff/12001/lily/stem.cc#newcode812
lily/stem.cc:812: Real beg = robust_scm2double (me->get_pure_property
("stem-begin-position", 0, INT_MAX), 0.0);
Using robust_scm2double does not help when me is 0.  This still needs
LY_ASSERT_TYPE or similar.

http://codereview.appspot.com/6584045/diff/12001/ly/engraver-init.ly
File ly/engraver-init.ly (right):

http://codereview.appspot.com/6584045/diff/12001/ly/engraver-init.ly#newcode1145
ly/engraver-init.ly:1145: \override Stem.stencil = #'()
Overriding a stencil with an empty list seems strange: is there a
difference in meaning to overriding it with #f?

http://codereview.appspot.com/6584045/diff/12001/ly/property-init.ly
File ly/property-init.ly (right):

http://codereview.appspot.com/6584045/diff/12001/ly/property-init.ly#newcode315
ly/property-init.ly:315: kievanOff = {
Could be just

    kievanOff = \undo \kievanOn

Less efficient (if we ever find it makes a difference, \undo could be
done in C++ mostly), but less prone to oversights as well.

http://codereview.appspot.com/6584045/