[Top][All Lists]

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

Re: Fix 1382 (issue3100041)

From: Carl . D . Sorensen
Subject: Re: Fix 1382 (issue3100041)
Date: Sun, 14 Nov 2010 14:17:19 +0000

On 2010/11/14 11:22:44, Valentin Villenave wrote:
Hi Carl,

looks good to me! I just have minor questions wrt coding style, but
these are
mainly because of my own ignorance :)
File input/regression/ (right):
input/regression/ \override StaffSymbol
#'staff-space =
Just a question: why did you chose to use "0.0" instead of plain "0"

Because that was in the original bug report.  It doesn't need to be
there; I can change it to just zero.
File lily/ (right):
lily/ Real denom =
Staff_symbol::staff_space (st);
Is there a reason why you're using "denom" instead of "denominator"?
From what I
can see Lily's source code doesn't use this abbrev. anywhere else...

Sometimes it uses the abbreviation "den" for denominator.  But the
current rule says we shouldn't use abbreviations.  I'll fix it.
lily/ st->warning (_ ("staff-space is 0,
staff-position to 0"));
I may be completely deluded here, but wouldn't it be cleaner to put
names as %d variables in the warning string? This could make it easier
localization, etc.) if we were to change the property names in the

I don't see how putting the names as variables helps anything.  The
whole string will need to be translated for localization, rather than
doing it one word at a time.

That being said, this warning generates a huge number of warnings, and
the code seems to do the right thing anyway, so I'm leaning toward
eliminating the warning.



reply via email to

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