You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fop-dev@xmlgraphics.apache.org by Jeremias Maerki <de...@jeremias-maerki.ch> on 2008/07/07 15:22:04 UTC
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=670341&view=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=670341&r1=670340&r2=670341&view=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
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
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=680378&view=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
Posted by Andreas Delmelle <an...@telenet.be>.
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
Posted by Adrian Cumiskey <ad...@gmail.com>.
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
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
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
Posted by "Peter B. West" <li...@pbw.id.au>.
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
Posted by Andreas Delmelle <an...@telenet.be>.
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
Posted by "Peter B. West" <pb...@pbw.id.au>.
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.
Re: svn commit: r670341 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo: FONode.java FOText.java FOTreeBuilder.java FObjMixed.java
Posted by Andreas Delmelle <an...@telenet.be>.
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
Posted by Max Berger <ma...@berger.name>.
-----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
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
On 07.07.2008 18:52:55 Andreas Delmelle wrote:
> 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?
No, breaking backwards compatibility remains a break. Authors of
extensions have to branch off new versions of their plug-ins. How many
times did we get annoyed when Batik changed their bridge APIs in a
non-backwards-compatible way? Even I had to restore backwards
compatibility in the renderers after some of my own changes because they
would have broken Barcode4J and I would have had to branch off a new
variant of the extension which easily leads to a support nightmare:
which extension do I need for which FOP version?
Am I the only one concerned about backwards-compatibility here?
> (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
Jeremias Maerki
Re: svn commit: r670341 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo: FONode.java FOText.java FOTreeBuilder.java FObjMixed.java
Posted by Andreas Delmelle <an...@telenet.be>.
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
Re: svn commit: r670341 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo: FONode.java FOText.java FOTreeBuilder.java FObjMixed.java
Posted by Andreas Delmelle <an...@telenet.be>.
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