Re: [PATCH] Implement new handling for margin and line-width settings.
On 8/13/09 11:21 AM, "Michael Käppler" wrote:
> Hi Carl,
>>> Am I right you suggest to modify the output-def just once but permanent?
>>>
>>
>> I meant to change the paper-def so that it would be consistent for the rest
>> of the processing (it would eliminate multiple warnings).
>>
> Okay. Anyway, I would prefer to not touch the original values. Instead
> we could introduce a new variable in the output-def, say
> "consistency-checked", set it to true after checking it the first time
> and don't touch it any more during processing.
> That would also be a good way to give the user control about whether his
> values are checked or not. If you set it to true in \paper {}, no
> checking will be done and no warnings printed.
> What do you say?
I don't know enough about this part of the LilyPond code to have a good idea
about the proper internals.
However, it would seem to me to be very un-LilyPond-ish to have a variable
consistencyChecked that it set to true by the program but is also accessible
to the user.
Rather, it would seem to me that there should be a property setting
checkMarginConsistency that would default to #t, but could be set #f. And
part of the check could be to set checkMarginConsistency to #f to prevent
further checks.
I realize this is only a small difference, but I think it's an important
one. LilyPond settings are based on what the user wants the program to do,
not based on what the program has already accomplished.
HTH,
Carl
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: [PATCH] Implement new handling for margin and line-width settings.
Carl Sorensen writes: > On 8/12/09 11:15 AM, "Michael Käppler" wrote: > >> Sorry, that wasn't intended. It was late yesterday and I didn't >> notice I pressed the wrong button... ;) > > Glad I'm not the only one who does this. Uh oh. Did you manage to placate her again? -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: [PATCH] Implement new handling for margin and line-width settings.
On 8/12/09 11:15 AM, "Michael Käppler" wrote:
> Hi Carl,
>> Usually we like to keep responses to reviews on the -devel list, so I've
>> copied this there.
>>
> Sorry, that wasn't intended. It was late yesterday and I didn't notice I
> pressed the wrong button... ;)
Glad I'm not the only one who does 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.
>>
> Am I right you suggest to modify the output-def just once but permanent?
I meant to change the paper-def so that it would be consistent for the rest
of the processing (it would eliminate multiple warnings).
>> I still think my suggestion for avoiding the variable consistency is a good
>> one. I have very rough pseudocode below
>>
> Done.
>> 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.
>>
> What do you want to achieve by this?
I want to avoid the multiple warnings you were concerned about.
> I don't really like the idea of
> mixing variables set in \paper {} with computed values from normalize ().
> The only aim of normalize () is to add missing values (although this
> isn't needed in some cases, e.g. right-margin is never used elsewhere)
> and set default values if the settings are considered bad.
> What I would like to add is the possibility to switch off this
> "consistency check", so that you can have negative margins and exceeding
> line-widths if you need to.
As long as the consistency check is enabled, you won't be using the paper{}
values anyway, so it seems to me that modifying them to create consistency
is no problem.
If you're planning to be able to disable the consistency check only for a
small portion of the output, then I could see that one would not want to
change the paper{} variables as part of the consistency check.
As I said, I don't have strong opinions about this. You can ignore this
suggestion if it doesn't make sense to you.
HTH,
Carl
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: [PATCH] Implement new handling for margin and line-width settings.
Hi Carl,
Usually we like to keep responses to reviews on the -devel list, so I've
copied this there.
Sorry, that wasn't intended. It was late yesterday and I didn't notice I
pressed the wrong button... ;)
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.
Am I right you suggest to modify the output-def just once but permanent?
I still think my suggestion for avoiding the variable consistency is a good
one. I have very rough pseudocode below
Done.
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.
What do you want to achieve by this? I don't really like the idea of
mixing variables set in \paper {} with computed values from normalize ().
The only aim of normalize () is to add missing values (although this
isn't needed in some cases, e.g. right-margin is never used elsewhere)
and set default values if the settings are considered bad.
What I would like to add is the possibility to switch off this
"consistency check", so that you can have negative margins and exceeding
line-widths if you need to.
Regards,
Michael
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: [PATCH] Implement new handling for margin and line-width settings.
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" 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,
default_line_width);
}
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,
default_line_width);
}
else
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
future.
Carl
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
