On 24.11.2008 13:14:35 Adrian Cumiskey wrote: > Jeremias, > > Jeremias Maerki wrote: > > Adrian, > > > > I'm unhappy with this change. I know you're constantly trying to > > improve things by refactoring but you also need to be careful about > > preserving compatibility. In this case, you broke my external PDF-in-PDF > > plug-in by changing the interface from "Class getSupportedImageClass()" > > to "Class[] getSupportedImageClasses()". Furthermore, chosing > > "ImageHandler" as the interface name will result in a name clash when > > the AFP branch is merged back into Trunk and I'll try to merge those > > changes into the IF branch. After all, in this branch the whole image > > handling is already properly abstracted for all renderers, not just PDF > > and AFP. So I think we need to revisit that before we consider a merge > > of the AFP GOCA branch into Trunk. > > > > I think, we have the following options: > > 1. Remove ImageHandler in the GOCA branch, restore plug-in compatibility > > for PDFImageHandler and have a separate service registry class for AFP > > and PDF. (My preffered option) > > I think this option would involve the most amount of work and regression > testing. Furthermore, this > is work that will probably be thrown away later. It does not need to be done > now and can be > revisited later when the Temp_AreaTreeNewDesign branch is ready to be merged > back into trunk. > > > 2. We can selectively merge parts of the IF branch (the ImageHandler > > abstraction) into trunk before merging the rest (I could do that for you, > > if nobody objects the partial merge-back). From there you could make use > > of it in the GOCA branch and restore the PDFImageHandler. You could then > > use the same image handlers for the current code as well as the > > implementation for the new IF. > > This sounds a bit messy, its better we do the merge in one go, a halfway > situation is never a good > one and I would expect quite a bit of management overhead created by this. > > > 3. I have to adjust my PDF plug-in and do another release knowing that > > this is only for restoring compatibility. > > I would favour this option. This represents the least amount of change to > FOP and the least amount > of work/regression testing etc. This is a very small change that should only > effect a small number > of users who upgrade their FOP installation to trunk and forget to upgrade > their pdf images plugin.
Cheap excuse to ignore backwards-compatibility. What if there were other people who wrote plug-ins of their own? > I have included the simple 2 line patch file for > src/java/org/apache/fop/render/pdf/pdfbox/PDFBoxPDFImageHandler.java. > Apologies if this creates a > little more work for you. Your patch has one problem: it kills backwards-compatibility for FOP 0.95. I won't delete the old method so this whole thing will still work with 0.95. The necessary changes in my plug-in are negligible. What isn't is the time I'll spend documenting which version of the plug-in the users have to select for which FOP version. Add to that time I spend on answering the questions of the people who don't RTFM. I wonder if you've spent one second thinking about backwards-compatibility before changing the PDFImageHandler interface. So I guess it's up to me anyway to fix this problem. Jeremias Maerki
