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.
