Bug 49703

Summary: [PATCH] resolve compilation and javadoc warnings
Product: XMLGraphicsCommons - Now in Jira Reporter: Glenn Adams <gadams>
Component: utilitiesAssignee: XML Graphics Project Mailing List <general>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: Trunk   
Target Milestone: --   
Hardware: All   
OS: All   
Attachments: Patch against revision 982145.

Description Glenn Adams 2010-08-04 04:11:30 UTC
This patch resolves all current compilation and javadoc warnings. Comments follow:

* deprecations on following were removed because the member name clearly indicates it is "old"; if a user wants to use something "old", then they shouldn't be warned of doing so:

  - XMPConstants.PDF_A_IDENTIFICATION_OLD
  - PDFAOldXMPSchema

* deprecation of drawString(String,...) on the TextHandler interface was removed due to newly added adapter classes (about which see below)

* new adapter classes TextHandlerAdapter and PSTextHandlerAdapter were added to provide default implementations of drawString(String,...)

* the existing StrokingTextHandler class was reworked to extend TextHandlerAdapter

* various javadocs build warnings were removed, some which entailed changing custom tag @todo to @asf.todo to avoid conflicts with future standardized extensions to javadoc tags

This patch was created against revision 982145 of the xmlgraphics commons trunk.

Regards,
Glenn
Comment 1 Glenn Adams 2010-08-04 04:13:24 UTC
Created attachment 25841 [details]
Patch against revision 982145.
Comment 2 Simon Pepping 2010-08-18 05:12:52 UTC
A few questions:

In the existing situation TextHandler deprecates registering a
Graphics2D object with the TextHandler object. Instead, the
recommended way is to give the Graphics2D object with the drawString
method. Why do you restore registering a Graphics2D object in its full
glory (remove deprecation, new method getGraphics)?

The PSTextHandlerAdapter is not used. Why do you introduce it?

There are now an interface PSTextHandler and a class
PSTextHandlerAdapter which does not implement that interface. The
names are confusing.
Comment 3 Glenn Adams 2010-08-18 05:43:59 UTC
Overall I was attempting to remove the deprecation warnings in FOP deriving from TextHandler#drawString. See inline below for specific answers.

(In reply to comment #2)
> A few questions:
> 
> In the existing situation TextHandler deprecates registering a
> Graphics2D object with the TextHandler object. Instead, the
> recommended way is to give the Graphics2D object with the drawString
> method. Why do you restore registering a Graphics2D object in its full
> glory (remove deprecation, new method getGraphics)?

The prior deprecation of the drawString method appeared to be an incomplete solution. In particular, StrokedTextHandler.java provided an implementation that continued to use a local reference to the G2D instance.
 
> The PSTextHandlerAdapter is not used. Why do you introduce it?
> 
> There are now an interface PSTextHandler and a class
> PSTextHandlerAdapter which does not implement that interface. The
> names are confusing.

I introduced it as a counterpart to the more generic TextHandlerAdapter in order to allow clients of the commons library to not encounter deprecation reports due to their use of PSTextHandler. However, I see I mistakenly had PSTextHandlerAdapter implementing TextHandler rather than PSTextHandler, which was my intent.

Generally speaking, with

TextHandlerAdapter
PSTextHandlerAdapter

I was trying to hide the deprecation of #drawString(String...) in external clients since the adapters provided an adequate solution.

Having said that, I am open to other solutions, such as leaving the original deprecation in place, but with a time schedule or specific release target for its removal.

G.
Comment 4 Jeremias Maerki 2010-09-02 11:20:28 UTC
I must say I'm not too happy with the idea of introducing multiple classes to handle a deprecation from almost two years ago (https://svn.apache.org/viewvc?view=revision&revision=718586) for an interface that has probably never been used directly outside of the XML Graphics project. Essentially, you're removing the deprecations completely. Without actually searching through the source code, the user won't find out anymore if he's working with a deprecated method.

We've had a release in between, so why not just remove this method? This is very technical stuff (i.e. not an end-user API) and we can expect people to migrate within 2 years should anyone have used it. But yes, freely upgrading xmlgraphics-commons.jar while staying with an older fop.jar will cause problems then. But at some point, deprecated stuff should be removed.

A similar argument for the "old" PDF/A identification URI. We're mostly about producing PDF here, not about reading it. So if anyone needs to process outdated PDF/A files they can recover the old PDF/A schema from out Subversion repo or write their own. At any rate, the old (wrong) URI should not be used anymore for new PDFs.

I've locally applied the patch and did the following changes to it:
- @asf.todo converted to //TODO comments
- Removed "old" PDF/A identification URI (deprecated)
- Removed org.apache.xmlgraphics.java2d.TextHandler.drawString(String, float, float); (deprecated)
Comment 5 Glenn Adams 2010-09-02 21:34:18 UTC
Thanks. Your resolution is a better one. I did not have the historical perspective to judge it was best to simple remove them.

G.
Comment 6 Jeremias Maerki 2010-09-09 06:04:43 UTC
No objections to the changes I made to Glenn's patch. I've applied the whole bunch:
http://svn.apache.org/viewvc?rev=995366&view=rev

Thanks, Glenn!!!