[Top][All Lists]

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

Re: [PATCH] Implement new handling for margin and line-width settings.

From: Carl Sorensen
Subject: Re: [PATCH] Implement new handling for margin and line-width settings.
Date: Tue, 11 Aug 2009 18:35:43 -0600

Thanks for the reply Michael.

Usually we like to keep responses to reviews on the -devel list, so I've
copied this there.

On 8/11/09 5:51 PM, "Michael Käppler" <address@hidden> wrote:

> Hi Carl,
> thanks for your review:
>>      Don't use lmargin, rmargin, and lwidth as variable names.  LilyPond
>> standards call for using full words in variable names.  You might want to
>> use something like scm_left_margin, scm_right_margin, and scm_line_width.
> Fixed. I used lmargin etc. because I saw it in layout->page-init as a
> temporary variable, therefore I thought there would be nothing against it.

Yes, we have old code that uses that kind of variable names.  As we rewrite,
we're supposed to go to full-word names.

>> I'd prefer the variable name consistent, instead of consistency
> Fixed.
>>> +
>>> + // Consistency checks. FIXME: Print warnings just once
>> This would be easy to add by putting the test in an if/elseif/endif block.
> Hmm... I don't really understand this. The problem is that
> foo->normalize () is called every time an Output_def is needed to get
> information about line-width and/or margins. So if one sets wrong values
> in \paper {} the function displays the error several times. But I
> decided to not overwrite the original paper settings during rendering,
> because maybe the original settings are later needed again. (However,
> I'm pretty unsure of this)

Oh -- I misunderstood the meaning of the FIXME.

I think I'd recommend changing the settings to something that is rational.
Since we're using the changed settings, we might as well change them so we
don't keep getting errors.

>> It might be cleaner to define a function set_layout_props (line_width,
>> left_margin, right_margin) and then call it with different arguments.  The
>> function call could be within the if/elseif/endif block described above,
>> and then there would be no need for the variable consistency.
> See above.

I still think my suggestion for avoiding the variable consistency is a good
one.  I have very rough pseudocode below

if (paper_width > (line_width + left_margin + right_margin))
     warning ("paper not wide enough for margins + line");
     set_paper_layout (default_left_margin, default_right_margin,
else if ((left_margin < 0) || (right_margin < 0))
     warning ("right or left margin has a negative value");
     set_paper_layout (default_left_margin, default_right_margin,
  set_paper_layout (left_margin, right_margin, line_width);

You could even be smarter in your choice of margins and line-widths to set.

For example, if left_margin > 0 and right_margin <0, you could do

set_paper_layout (left_margin, line_width + right_margin, 0)

or something like that.

None of my suggestions are mandatory.  They're just suggestions.

Thanks for taking on this task!  It will be good for LilyPond users in the


reply via email to

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