Re: Adds longas, maximas and non-standard tweaks to MultiMeasureRest (issue4536068)

[dak]

I would suggest reverting this patch as "needs work" for now.


http://codereview.appspot.com/4536068/diff/30001/lily/multi-measure-rest.cc
File lily/multi-measure-rest.cc (right):

http://codereview.appspot.com/4536068/diff/30001/lily/multi-measure-rest.cc#newcode125
lily/multi-measure-rest.cc:125: SCM sml = dynamic_cast<Spanner *>
(me)->get_bound (LEFT)
There is no good way to get around the cast?

http://codereview.appspot.com/4536068/diff/30001/lily/multi-measure-rest.cc#newcode130
lily/multi-measure-rest.cc:130: double duration_log = -log2
(ml.Rational::to_double ());
log2 is a floating point operation not guaranteed to be exact.  One
usually uses a loop for figuring out a proper integer value.  It is also
not clear to me that this will pick out the right kind of rest always.

For example, what to do about 3/2?  We either get 2/1 rests, or 1/1
rests, and it is not clear to me that this rounding choice is
necessarily the same as that for 3/4.

I think it might be saner to just have an overrideable lookup list for
exact meters (and 4/4 is not necessarily the same as 2/2 here), and
revert to a separately configurable default otherwise, likely a whole
rest.

http://codereview.appspot.com/4536068/diff/30001/lily/multi-measure-rest.cc#newcode140
lily/multi-measure-rest.cc:140: int list_elt = scm_to_int (scm_list_ref
(duration_logs_list, scm_from_int (i)));
Iterating through a list as if it were an array is a no-no.  This makes
the operation O(n^2) instead of O(n) and obfuscates what happens.  Check
other code examples for how to iterate through a list.

http://codereview.appspot.com/4536068/diff/30001/lily/multi-measure-rest.cc#newcode229
lily/multi-measure-rest.cc:229: for (int i = 0; i < scm_to_int
(scm_length (duration_logs_list)); i++)
Again: don't loop through a list by indexing it like an array.

http://codereview.appspot.com/4536068/diff/30001/lily/multi-measure-rest.cc#newcode246
lily/multi-measure-rest.cc:246: length = int (pow (2, -i));
Don't use floating point arithmetic.  This is just 2<<-i unless i is
positive.  One should exit the loop _before_ that happens, otherwise
length will be undefined with the integer variant.  With the floating
point variant, length becomes 0 before exiting the loop with i==1.  I
doubt that is intentional.

http://codereview.appspot.com/4536068/diff/30001/lily/multi-measure-rest.cc#newcode254
lily/multi-measure-rest.cc:254: Stencil r (musfont->find_by_name
("rests." + to_string (k)));
This may calculate a name "rests.-1" instead of the valid "rests.M1".
Use Rest::glyph_name instead, though it may also need fixing in that
regard.

http://codereview.appspot.com/4536068/