Re: svn commit: r670341 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo: FONode.java FOText.java FOTreeBuilder.java FObjMixed.java
Almost forgot this but then again stumbled over the left-over bugs a couple of times. So this is fixed now as discussed: http://svn.apache.org/viewvc?rev=680378view=rev As long as external extensions derive from FONode or XMLObj everything should work as before. Otherwise, the change might still cause problems when super.characters() isn't called by a subclass of FONode. FONode.addCharacters() is now deprecated. On 09.07.2008 19:42:24 Andreas Delmelle wrote: On Jul 9, 2008, at 11:06, Jeremias Maerki wrote: On 09.07.2008 10:45:35 Max Berger wrote: My favorite solution would be: Provide the new semantics with a new signature (or method name), and keep the old one as deprecated for at least 1 release (Then all plugin developers have enough time to adjust), then remove it. +1 to that approach. I volunteer to do the necessary changes if we reach a consensus. Well, why not? Seems like a good compromise. Have the old method compute the end-index off the passed length, and delegate the call the new version... Then you can actually /see/ how silly it looked, and why I decided to make the change in the first place. The end-result will be the same, only it will take a few more months... In the meantime, I'll see if I can pass you some other info about the future. ;-) Cheers Andreas Jeremias Maerki
Re: svn commit: r670341 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo: FONode.java FOText.java FOTreeBuilder.java FObjMixed.java
Andreas Delmelle wrote: On Jul 9, 2008, at 09:39, Peter B. West wrote: Jeremias Maerki wrote: Am I the only one concerned about backwards-compatibility here? It's not my *concern*, but deliberately breaking compatibility does seem pretty silly. Yeah, so one night I thought: Let's see if we can annoy everyone who has the bad habit of not using the readily provided base classes for extensions... :- Cheers Andreas That explains it! -- Peter B. West http://cv.pbw.id.au/ Folio http://defoe.sourceforge.net/folio/
Re: svn commit: r670341 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo: FONode.java FOText.java FOTreeBuilder.java FObjMixed.java
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Jeremias, Jeremias Maerki schrieb: Am I the only one concerned about backwards-compatibility here? No. I am also concerned about backwards-compatibility, but in a different way: This change changed the semantics without changing the API, therefore code still compiled, but crashed (such as the bug I encountered). This is a type of api change I am not happy with. What would be ok with me is if the interface had changed (in this case, the signature of the functions). My code would no longer compile, and I'd have to prepare a new plugin for the new version (which i currently have to do anyways). My favorite solution would be: Provide the new semantics with a new signature (or method name), and keep the old one as deprecated for at least 1 release (Then all plugin developers have enough time to adjust), then remove it. http://java.sun.com/j2se/1.5.0/docs/guide/javadoc/deprecation/deprecation.html Max -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFIdHqv+Gr+4pk71JwRAoQRAJ9stE0yyjmyBv4eebTru4CDiwSfqgCcCJWI VTbbnu86aZTMT1/sro8/c2c= =u05W -END PGP SIGNATURE-
Re: svn commit: r670341 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo: FONode.java FOText.java FOTreeBuilder.java FObjMixed.java
On 09.07.2008 10:45:35 Max Berger wrote: Jeremias, Jeremias Maerki schrieb: Am I the only one concerned about backwards-compatibility here? No. I am also concerned about backwards-compatibility, but in a different way: This change changed the semantics without changing the API, therefore code still compiled, but crashed (such as the bug I encountered). This is a type of api change I am not happy with. What would be ok with me is if the interface had changed (in this case, the signature of the functions). My code would no longer compile, and I'd have to prepare a new plugin for the new version (which i currently have to do anyways). My favorite solution would be: Provide the new semantics with a new signature (or method name), and keep the old one as deprecated for at least 1 release (Then all plugin developers have enough time to adjust), then remove it. +1 to that approach. I volunteer to do the necessary changes if we reach a consensus. http://java.sun.com/j2se/1.5.0/docs/guide/javadoc/deprecation/deprecation.html Max Jeremias Maerki
Re: svn commit: r670341 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo: FONode.java FOText.java FOTreeBuilder.java FObjMixed.java
Sounds like a fine approach, +1 from me. Adrian. Jeremias Maerki wrote: On 09.07.2008 10:45:35 Max Berger wrote: Jeremias, Jeremias Maerki schrieb: Am I the only one concerned about backwards-compatibility here? No. I am also concerned about backwards-compatibility, but in a different way: This change changed the semantics without changing the API, therefore code still compiled, but crashed (such as the bug I encountered). This is a type of api change I am not happy with. What would be ok with me is if the interface had changed (in this case, the signature of the functions). My code would no longer compile, and I'd have to prepare a new plugin for the new version (which i currently have to do anyways). My favorite solution would be: Provide the new semantics with a new signature (or method name), and keep the old one as deprecated for at least 1 release (Then all plugin developers have enough time to adjust), then remove it. +1 to that approach. I volunteer to do the necessary changes if we reach a consensus. http://java.sun.com/j2se/1.5.0/docs/guide/javadoc/deprecation/deprecation.html Max Jeremias Maerki
Re: svn commit: r670341 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo: FONode.java FOText.java FOTreeBuilder.java FObjMixed.java
On Jul 9, 2008, at 09:26, Jeremias Maerki wrote: Am I the only one concerned about backwards-compatibility here? Not really. There's always Microsoft... :-) Seriously, if I had made the change in 0.95, I would completely agree (and would probably already have reverted the change). Since it's in Trunk, however, is still quite some time away from being released. On another note, if extension implementors use ExtensionObj or XMLObj, they have nothing to worry about. Only the very few that subclass FONode directly need to watch out. Andreas
Re: svn commit: r670341 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo: FONode.java FOText.java FOTreeBuilder.java FObjMixed.java
On Jul 9, 2008, at 09:39, Peter B. West wrote: Jeremias Maerki wrote: Am I the only one concerned about backwards-compatibility here? It's not my *concern*, but deliberately breaking compatibility does seem pretty silly. Yeah, so one night I thought: Let's see if we can annoy everyone who has the bad habit of not using the readily provided base classes for extensions... :- Cheers Andreas
Re: svn commit: r670341 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo: FONode.java FOText.java FOTreeBuilder.java FObjMixed.java
On Jul 9, 2008, at 11:06, Jeremias Maerki wrote: On 09.07.2008 10:45:35 Max Berger wrote: My favorite solution would be: Provide the new semantics with a new signature (or method name), and keep the old one as deprecated for at least 1 release (Then all plugin developers have enough time to adjust), then remove it. +1 to that approach. I volunteer to do the necessary changes if we reach a consensus. Well, why not? Seems like a good compromise. Have the old method compute the end-index off the passed length, and delegate the call the new version... Then you can actually /see/ how silly it looked, and why I decided to make the change in the first place. The end-result will be the same, only it will take a few more months... In the meantime, I'll see if I can pass you some other info about the future. ;-) Cheers Andreas
Re: svn commit: r670341 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo: FONode.java FOText.java FOTreeBuilder.java FObjMixed.java
I know I'm late on this one but I've only just stumbled over it while
playing with the AFP renderer in the GOCA branch. This change is very
dangerous as it essentially breaks every FOP extension that uses
character content, especially those not developed inside the FOP project.
I'm lucky it doesn't (shouldn't) break Barcode4J but I would strongly
suggest to revert this interface change especially since the method
signature doesn't change while the semantics do.
On 22.06.2008 13:18:03 adelmelle wrote:
Author: adelmelle
Date: Sun Jun 22 04:18:03 2008
New Revision: 670341
URL: http://svn.apache.org/viewvc?rev=670341view=rev
Log:
Changed FONode.addCharacters() signature to match the characters() event (use
'length' as a parameter instead of 'end')
Modified:
xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/FONode.java
xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/FOText.java
xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/FOTreeBuilder.java
xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/FObjMixed.java
Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/FONode.java
URL:
http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/FONode.java?rev=670341r1=670340r2=670341view=diff
==
--- xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/FONode.java (original)
+++ xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/FONode.java Sun Jun 22
04:18:03 2008
@@ -274,12 +274,12 @@
*
* @param data array of characters containing text to be added
* @param start starting array element to add
- * @param end ending array element to add
+ * @param length number of elements to add
* @param pList currently applicable PropertyList
* @param locator location in the XSL-FO source file.
* @throws FOPException if there's a problem during processing
*/
-protected void addCharacters(char[] data, int start, int end,
+protected void addCharacters(char[] data, int start, int length,
PropertyList pList,
Locator locator) throws FOPException {
// ignore
snip/
Jeremias Maerki
Re: svn commit: r670341 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo: FONode.java FOText.java FOTreeBuilder.java FObjMixed.java
On Jul 7, 2008, at 15:22, Jeremias Maerki wrote: I know I'm late on this one but I've only just stumbled over it while playing with the AFP renderer in the GOCA branch. This change is very dangerous as it essentially breaks every FOP extension that uses character content, especially those not developed inside the FOP project. I'm lucky it doesn't (shouldn't) break Barcode4J but I would strongly suggest to revert this interface change especially since the method signature doesn't change while the semantics do. I'd rather keep it the other way around, like it is now. It's much less confusing if the same parameters are used as in the SAX characters() event. If this breaks external code, then my apologies (it even broke some internal classes too, but those issues didn't show when running the test-suite; Max discovered them). Still -1 for reverting. Cheers Andreas
Re: svn commit: r670341 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo: FONode.java FOText.java FOTreeBuilder.java FObjMixed.java
On Jul 7, 2008, at 18:09, Andreas Delmelle wrote: On Jul 7, 2008, at 15:22, Jeremias Maerki wrote: I know I'm late on this one but I've only just stumbled over it while playing with the AFP renderer in the GOCA branch. This change is very dangerous as it essentially breaks every FOP extension that uses character content, especially those not developed inside the FOP project. I'm lucky it doesn't (shouldn't) break Barcode4J but I would strongly suggest to revert this interface change especially since the method signature doesn't change while the semantics do. I'd rather keep it the other way around, like it is now. It's much less confusing if the same parameters are used as in the SAX characters() event. If this breaks external code, then my apologies (it even broke some internal classes too, but those issues didn't show when running the test-suite; Max discovered them). Still -1 for reverting. Note: I do appreciate the feedback, and I see where this can become problematic, but OTOH, if there is a class that relies on the ending index being passed, the solution is rather straightforward. The change was ultimately also motivated by the simple question: Why did we need to compute the end-index off start and length for every characters() event? The answer: Because FONode.addCharacters() expected it. That's the only reason, so it made more sense to simply make it a length (no additional operation needed) and only compute the end index if we really need it... That said: Would it relieve your concerns a bit if this change were better documented? (Which I'd be glad to take care of, since I neglected to do so in spite of the change being so high up in the hierarchy /and/ exposed to potential subclasses... :/) Andreas
