Implement new handling for \paper margin settings.

Implement new handling for \paper margin settings.

[Carl, 2009-08-20 12:54:08]

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
  }

[Neil Puttock, 2009-08-21 00:04:22]

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
On 2009/08/20 12:54:08, Carl wrote:
> 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
>   }
> 

How about using scm_is_number () instead?

http://codereview.appspot.com/109051/diff/1/5#newcode156
Line 156: programming_error (_ ("called normalize() on paper with missing
settings"));
space after normalize

programming errors aren't localized

http://codereview.appspot.com/109051/diff/1/5#newcode171
Line 171: ? scm_to_double(scm_right_margin)
space after scm_to_double

http://codereview.appspot.com/109051/diff/1/5#newcode192
Line 192: if (c_variable ("check-consistency") == SCM_BOOL_T)
could use to_boolean here

if (to_boolean (c_variable ("check-consistency")))

http://codereview.appspot.com/109051/diff/1/5#newcode200
Line 200: warning (_ ("margins don't fit with line-width, setting default
values"));
don't -> do not

http://codereview.appspot.com/109051/diff/1/5#newcode211
Line 211: set_variable (ly_symbol2scm ("left-margin"),
scm_from_double(left_margin));
space after scm_from_double

(same for following lines)

http://codereview.appspot.com/109051/diff/1/7
File ly/paper-defaults-init.ly (left):

http://codereview.appspot.com/109051/diff/1/7#oldcode45
Line 45: between-scores-system-spacing = #'((space . 14) (minimum-distance . 8)
(padding . 1))
restore