Re: Approximates cross-staff slurs in VerticalAxisGroup vertical-skylines. (issue 6498077)

[dak]

http://codereview.appspot.com/6498077/diff/21/lily/axis-group-interface.cc
File lily/axis-group-interface.cc (right):

http://codereview.appspot.com/6498077/diff/21/lily/axis-group-interface.cc#newcode390
lily/axis-group-interface.cc:390: /*
Where is the point in putting a whole callback inside of a comment?  Is
this a TODO "make callback work again"?  Or what?  Of course, using /*
*/ for outcommenting the whole callback relies on no single comment ever
appearing inside of the whole function since comments don't nest.

I have no doubt that this is the predominant coding style in LilyPond,
and a syntax error is a small punishment for violating this convention,
but the usual (and nestable) way of outcommenting a part of source is to
use #if 0.  As well as an explanatory comment just _why_ things are
outcommented instead of removed.

http://codereview.appspot.com/6498077/diff/21/lily/axis-group-interface.cc#newcode419
lily/axis-group-interface.cc:419: */
See above.

http://codereview.appspot.com/6498077/diff/21/lily/page-layout-problem.cc
File lily/page-layout-problem.cc (right):

http://codereview.appspot.com/6498077/diff/21/lily/page-layout-problem.cc#newcode1004
lily/page-layout-problem.cc:1004: if (is_spaceable (staves[i]))
This indentation is simply wrong and does not match the program
structure.

http://codereview.appspot.com/6498077/diff/21/lily/phrasing-slur-engraver.cc
File lily/phrasing-slur-engraver.cc (right):

http://codereview.appspot.com/6498077/diff/21/lily/phrasing-slur-engraver.cc#newcode119
lily/phrasing-slur-engraver.cc:119: if (slur_stubs_.find (slurs_[j]) ==
slur_stubs_.end ())
It is quite nonsensical to have slur_stubs be a map indexed via
slurs_[j] rather than just an array indexed through j.

That is inefficient use of time, memory, and complexity.

http://codereview.appspot.com/6498077/diff/21/lily/phrasing-slur-engraver.cc#newcode332
lily/phrasing-slur-engraver.cc:332: map<Grob *, Grob *> vag_to_slur;
Again: why a map here?

http://codereview.appspot.com/6498077/diff/21/lily/script-column.cc
File lily/script-column.cc (right):

http://codereview.appspot.com/6498077/diff/21/lily/script-column.cc#newcode49
lily/script-column.cc:49: ? 1
? 1 : 0 is totally unnecessary since || already returns 1 or 0.

http://codereview.appspot.com/6498077/diff/21/lily/script-column.cc#newcode60
lily/script-column.cc:60: if (unsmob_grob (i1->get_object ("slur")) &&
unsmob_grob (i2->get_object ("slur")))
If both scripts have a field "slur" that is a grob, return #t is the
avoid-slur property of the second script is "outside" or "around" while
that of the first is neither.

Why don't you write this in a comment?  Why this complex code?  And what
is the ultimate purpose?

http://codereview.appspot.com/6498077/diff/21/lily/slur-engraver.cc
File lily/slur-engraver.cc (right):

http://codereview.appspot.com/6498077/diff/21/lily/slur-engraver.cc#newcode56
lily/slur-engraver.cc:56: map<Grob *, vector<Grob *> > slur_stubs_;
see comments in phrasing slur engraver.

http://codereview.appspot.com/6498077/diff/21/lily/slur.cc
File lily/slur.cc (right):

http://codereview.appspot.com/6498077/diff/21/lily/slur.cc#newcode623
lily/slur.cc:623: "thickness "
No entry for "surrogate"?

http://codereview.appspot.com/6498077/