Re: Make default margin values depend on paper size.

[Michael Käppler]

Hi Carl,
thanks for your review.
> But the things they stand for are symbols elsewhere in the code, so I'd
> prefer using symbols here.
Sure, but...
> http://codereview.appspot.com/115065/diff/1001/1003#newcode232
> Line 232: ((value-symbol (string->symbol (string-append (car value)
> "-default")))
> I you were using symbols, you'd have
>
> (value-symbol
>   (string->symbol (string-append
>                        (symbol->string (car value))
>                        "-default"))) 
... this conversion is the reason I dislike this. I have to append the 
suffix first, so in my opinion it's better to store them as strings.
> http://codereview.appspot.com/115065/diff/1001/1003#newcode221
> Line 221: (cons "left-margin" w)
> I think you could alternatively do (either with symbols or strings):
>
> (scaleable-values `((left-margin . ,w)
>                                     (right-margin . ,w)
>                                     (top-margin . ,h)
>                                     ...
>                                     (short-indent . ,w))
Done.
> http://codereview.appspot.com/115065/diff/1001/1003#newcode244
> Line 244: (module-define! m 'left-margin-default-scaled (cdr (assoc
> "left-margin" scaled-values)))
> This is a potential error waiting to cause problems for  a user.  assoc
> can return #f, and (cdr #f) is an error.
I don't think this is a problem here, because the scaleable-values list 
isn't intended to be changed by the user. But there's another problem I 
fixed now: the old version would crash if the default value isn't 
defined in paper-defaults-init.ly or elsewhere. Now it's set to zero in 
this case and a programming error is outputted.

Regards,
Michael