DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUGĀ·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=41044>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED ANDĀ·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=41044





------- Additional Comments From [EMAIL PROTECTED]  2007-06-25 15:39 -------
Hi Richard,

I've finally found some time to finish reviewing your patch, and it looked good 
at first glance, apart 
from some style issues (javadocs, bracing conditional branches even if there's 
only one statement, 
blabla) and what I take to be some small typos (for example: in 
CommonHyphenation.equals(), you 
compared the country, language and script properties to themselves instead of 
to their counterparts 
from the other instance).

Unfortunately, when I ran the junit tests, there seems to be a lot of work 
before we can apply it as is... I 
get 70 errors. :/ (a lot of them NPEs)
Am I doing something wrong, or did you skip the testsuite?

The only thing I really threw out were the deprecated-tags from 
CompoundDataType and 
LengthRangeProperty. This does need some further thought, nevertheless.
For CommonBorderPaddingBackground.setBorderInfo(), I agreed. So much even that 
I removed 
setBorderInfo() and setPadding() completely, instead of merely deprecating.

For now, I'll offer a bit more info on the rationale behind the mutability of 
compounds, and an idea on 
how to tackle it.
Compounds would be tricky in terms of canonicalization, because, in order to 
judge which compound 
instance you need, you have to have all the components available (e.g.: for 
space-before you need 
the .minimum, .maximum, .optimum, .conditionality and .precedence components).
This means that, currently, in order to correctly canonicalize a compound, we 
would have to wait until 
all attributes for a given FO have been converted to properties. We cannot do 
so at the time the 
compound is first created, since it will at that point only have the specified 
value of /one/ of the 
components (the first one in the list)

The sole reason why they are mutable is that they are not created in one go, 
but rather, the base-
property is created once the first component is encountered in the 
attribute-list. Further on in the 
process, as the other components are converted, the PropertyMakers need to be 
able to change the 
base-property (through setComponent())

A possible solution I'm thinking of is to rewrite part of the 
convertAttributeToProperty-loop in 
PropertyList. If we can make sure that compounds are always created as a single 
compound property 
(instead of the multiple components each setting the component of a 
base-property), then they would 
not need to be mutable at all, and the issue is rendered moot. 
FWIW, I've always disliked a bit the fact that the attributes are simply looped 
over. The hacks a bit 
higher up, in addAttributesToList(), for extracting column-number and font-size 
before all others, are 
to me an indication that this should be revisited.

As I roughly see it ATM, a compound would be made/instantiated using a list of 
attributes, instead of 
only one. As soon as one component is encountered, we would immediately scan 
the Attributes, 
extracting all the other components, and can then pass them all into the 
compound's PropertyMaker. As 
such, there is no more need for the setComponent() methods, since by converting 
one component's 
attribute, we would immediately convert the whole compound.

WDYT?

Andreas

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

Reply via email to