LGTM.  I do have one comment about tests for != SCM_UNDEFINED.  It think
there's less chance of confusion when testing for "undefined" rather
than "not undefined", which is "defined".  The double negative (not
undefined) can potentially be confusing.

But this is just my opinion, and you can feel free to ignore it.

Carl


http://codereview.appspot.com/109051/diff/1/5
File lily/output-def.cc (right):

http://codereview.appspot.com/109051/diff/1/5#newcode146
Line 146: if (scm_paper_width != SCM_UNDEFINED
I'd prefer to see this and all of your checks for SCM_UNDEFINED) be
written as
if (scm_paper_width = SCM_UNDEFINED) ||
    (scm_left_margin = SCM_UNDEFINED) ||
    (scm_right_margin = SCM_UNDEFINED)
  {
     Print error message
  }
else
  {
    Handle proper settings
  }

http://codereview.appspot.com/109051


_______________________________________________
lilypond-devel mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/lilypond-devel

Reply via email to