Finn, thanks a lot for looking at this.
On Tue, 30 Aug 2005 03:19 am, Finn Bock wrote: > [Jeremias Maerki on ] > > > Yeah: Wow! Finn, you're the specialist here. Can you take the lead > > on this one? > > I'm hardly a specialist on the layout system, and that is where a > good sized part of Manuels patch is, but in the property system I do > not like the hack to LineHeightPropertyMaker and > FontSizePropertyMaker that test for a PercentLength since it will not > work for expressions. I assume that a FixedLength is returned because > somebody > (BlockLM.createLineManager?) tries to resolve line-height before a > context is available. The solution is to delay resolution of > line-height, by passing the Length into the LineLM. Agree, and I have a solution for that ready to go. > > Eventually the no-args getValue() and getNumericValue() should be > removed. And a throws PropertyException added to the > Length.getValue() signature. But that is for later. There are quite a few "legal" situations where a context is not needed. It is a arguable if calling getValue(null) is superior to getValue(). > > In the layout system, I find to strange to see calls to the > initialize() method from addAreas() and getParentArea(). I would > rather see a public initialize() method that was called once from the > outside, perhaps from AbstractLayoutManager.getChildLM() before a LM > is traversed by getNextKnuthElements. But not all agree: > > http://marc.theaimsgroup.com/?l=fop-dev&m=111811772615007&w=2 > In this case I went with what was there. > > Other than that, the patch looks ok. > > > There is one thing I would have made differently: Instead of setting > an PercentBase.XXX integer on the PropertyMakers I would set an > instance of an implementation of PercentBase that implemented the > rule by that property. So there would be a subclass of PercentBase > for each of the rules you have identified in the > PropertyHandling/Percentages wiki. That way the code for dealing with > percentage rules can be placed in the property package instead of the > LM package. The LM package only supplied the context that the rules > can work on. But since I haven't written that code, I don't get to > decide. > I did consider that. One reason I didn't do that was to avoid cross "linking" the packages. The rules which require navigation to an ancestor do in this implementation require access to methods in the LMs. By having this in the Property packages would mean importing LM stuff into the Property packages. Something I was trying to avoid. > > regards, > finn