Jeremias Maerki wrote:
On 24.11.2008 13:14:35 Adrian Cumiskey wrote:
Jeremias Maerki wrote:
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.
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)
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.
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.
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.
3. I have to adjust my PDF plug-in and do another release knowing that
this is only for restoring compatibility.
Cheap excuse to ignore backwards-compatibility. What if there were other
people who wrote plug-ins of their own?
I'm sure if such a person existed and they wrote their own FOP plug-in they could very easily fix a
very small compatibility issue such as this.
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
I don't see how this would at all affect 0.95 compatibility. You already
provide a cersion 1.2
(http://www.jeremias-maerki.ch/download/fop/pdf-images) which will work just
fine for 0.95
installations - so no change there. And you also already provide a version 1.1a (from rev 611768 or
later), so all that needs to be done once the Temp_AFPGOCAResources branch is merged back into trunk
is release a new trunk version with the 2 line patch applied.
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 sympathize, users can be demanding even when you give of your time freely :).
I wonder if you've spent one second thinking about
backwards-compatibility before changing the PDFImageHandler interface.
Quite simply I wasn't aware that your external pdf images package touched upon this interface. Its
not part of the FOP code base. I will continue to always want what is best for the health of the
project, I find your comment both unnecessarily rude and disrespectful.