[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Make default margin values depend on paper size.

From: Michael Käppler
Subject: Re: Make default margin values depend on paper size.
Date: Tue, 08 Sep 2009 14:54:02 +0200
User-agent: Thunderbird (X11/20071114)

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...
Line 232: ((value-symbol (string->symbol (string-append (car value)
I you were using symbols, you'd have

  (string->symbol (string-append
                       (symbol->string (car value))
... 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.
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))
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 or elsewhere. Now it's set to zero in this case and a programming error is outputted.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]