This work looks good to me, but I'm not an expert in this area.
I have one question, I think. Right now, the alteration consists of two integers, which have implied denominators of 1/2 and 1/4, if I understand correctly. Would it be more general to have the alteration consist of two rationals? Or could we have some way of defining a base-alteration pair that would be rationals? And then the implied denominators of 1/2 and 1/4 would no longer be implicit, but would be explicit in either the alteration or the base-alteration? I think (but am not sure) that such an implementation would meet the needs that Hans Aberg has defined for appropriate microtonal support. Of course, this entire suggestion could be garbage, because I am not an expert on this topic, as I mentioned above. Thanks, Carl http://codereview.appspot.com/3789044/diff/29001/lily/accidental-placement.cc File lily/accidental-placement.cc (right): http://codereview.appspot.com/3789044/diff/29001/lily/accidental-placement.cc#newcode239 lily/accidental-placement.cc:239: Alteration last_alteration (0); Why is this last_alteration (0), instead of last_alteration () or last_alteration (NATURAL)? http://codereview.appspot.com/3789044/diff/29001/lily/include/pitch.hh File lily/include/pitch.hh (right): http://codereview.appspot.com/3789044/diff/29001/lily/include/pitch.hh#newcode60 lily/include/pitch.hh:60: Pitch (int octave, int notename, int alt1, int alt2); I'm somewhat hesitant about this code that limits an alteration to two integers. Are we sure that this is general enough given the fact that we're moving to an Alteration datatype? http://codereview.appspot.com/3789044/diff/29001/lily/scale.cc File lily/scale.cc (right): http://codereview.appspot.com/3789044/diff/29001/lily/scale.cc#newcode36 lily/scale.cc:36: " arguments are rational numbers giving the weigth in" spelling: weigth -> weight http://codereview.appspot.com/3789044/diff/29001/lily/scale.cc#newcode94 lily/scale.cc:94: " scale. The number of pitches in this scale minus one" "The number of scale steps in an octave is the number of pitches in the global scale minus one." http://codereview.appspot.com/3789044/diff/29001/python/musicexp.py File python/musicexp.py (right): http://codereview.appspot.com/3789044/diff/29001/python/musicexp.py#newcode308 python/musicexp.py:308: return '(ly:make-pitch %d %d \'(%d . 0))' % (self.octave, Why is the 0 for the second alteration hard-coded? http://codereview.appspot.com/3789044/diff/29001/scm/define-markup-commands.scm File scm/define-markup-commands.scm (right): http://codereview.appspot.com/3789044/diff/29001/scm/define-markup-commands.scm#newcode2418 scm/define-markup-commands.scm:2418: (interpret-markup layout props (markup #:musicglyph (assoc-get '(2 . 0) standard-alteration-glyph-name-alist "")))) Can this use SHARP, instead of (2 . 0)? http://codereview.appspot.com/3789044/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel