Re: Allow override of NoteHead.ledger-positions (issue 295970043 by address@hidden)

[paulwmorris]

Patch set 2, based on David's Feedback.
-Paul


https://codereview.appspot.com/295970043/diff/1/lily/ledger-line-spanner.cc
File lily/ledger-line-spanner.cc (right):

https://codereview.appspot.com/295970043/diff/1/lily/ledger-line-spanner.cc#newcode221
lily/ledger-line-spanner.cc:221: int len = scm_to_int (scm_length
(posns));
On 2016/04/22 08:17:09, dak wrote:
> scm_ilength already does scm_to_int.

Ah, thanks for the tip.

> But if you are getting the size of the
> vector in advance anyway (rather than just do {...} while (scm_is_pair
> (posns));), why don't you dimension the vector in advance?

Ok, thanks, I changed it to set the vector size and switched to using
do/while.  But then I thought there must be a helper function for this,
and sure enough there is ly_scm2floatvector in lily-guile.cc, so I've
used that in patch set 2.

> But at any rate, wouldn't it be better to just pass an optional grob
argument to
> Staff_symbol::ledger_positions so that this function will extract the
> ledger-positions preferably from this extra argument?  That way the
extraction
> code is all in one place.

That makes sense to me.  I finally got the argument passing working (as
an optional const pointer) ...after a few maddening rounds wrestling
with C++.  Please let me know if there's a better way than what I did.

https://codereview.appspot.com/295970043/