Re: svn commit: r1177251 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fo/flow/BasicLink.java layoutmgr/inline/InlineLayoutManager.java pdf/PDFFactory.java
i just checked this change against the complex script branch, and it's ok (doesn't regress); so if you want to leave it in, i won't object, though, in general, i prefer the "if it ain't broke, don't fix it" rule; i would also note that in the complex script branch i had previously redefined BidiOverride to inherit from Inline in order to reuse a portion of its implementation; i would not want to redefine it again merely to follow the same path you have chosen for BasicLink; also, just to note further, your change seems to have introduced a new findbugs warning; perhaps you can fix: CodeWarning UPMPrivate method org.apache.fop.layoutmgr.inline.InlineLayoutManager.getInlineFO() is never called Bug type UPM_UNCALLED_PRIVATE_METHOD (click for details) In class org.apache.fop.layoutmgr.inline.InlineLayoutManager In method org.apache.fop.layoutmgr.inline.InlineLayoutManager.getInlineFO() At InlineLayoutManager.java:[line 111] On Fri, Sep 30, 2011 at 10:54 PM, Glenn Adams wrote: > let's step back a minute; what was the problem you were trying to solve? > what was broken? how did your change fix it? > > if you made this change just because you think instanceof Inline should > return false on a BasicLink, then this change would seem gratuitous > > it is wholly reasonable that BasicLink may share the implementation of > Inline as previously held; your change required you to copy/paste existing > code from Inline into BasicLink and to alter InlineLayoutManager for no > purpose other than accommodating your change > > IMO you should revert the change > > On Fri, Sep 30, 2011 at 9:18 PM, Peter Hancock wrote: > >> By ancestor I refer to the relationship with respect to the fo: >> element hierarchy: Since the definition of fo:basic-link does not >> depend upon fo:inline, we cannot conclude that fo:basic-link is an >> fo:inline. >> >> The parameter entity "%inline;" refers to inline-level fo elements, >> fo:inline and fo:basic-link being members, and this is now reflected >> on the FOP FO object hierarchy, where Inline and BasicLink extend >> InlineLevel >> >> Have I understood the recommendation correctly, or have I missed anything? >> >> On Fri, Sep 30, 2011 at 1:18 PM, Glenn Adams wrote: >> > i'm not sure what you mean by 'ancestor', since containment relation is >> not >> > at issue here; >> > your argument is counter to the definition of the parameter entity >> %inline; >> > defined in XSL 1.1 Section 6.2 >> > >> > The parameter entity, "%inline;" in the content models below, contains >> the >> > following formatting objects: >> > >> > bidi-override >> > character >> > external-graphic >> > instream-foreign-object >> > inline >> > inline-container >> > leader >> > page-number >> > page-number-citation >> > page-number-citation-last >> > scaling-value-citation >> > basic-link >> > multi-toggle >> > index-page-citation-list >> > >> > i believe you should first restore the previous state of affairs, and >> then, >> > if you wish to continue making the case that it is not inline, take it >> up >> > with the group and get consensus before making what appears to be a >> possibly >> > unjustified architectural change >> > >> > On Fri, Sep 30, 2011 at 5:31 PM, Peter Hancock > > >> > wrote: >> >> >> >> While fo:basic-link and fo:inline are both inline level elements, one >> >> is not the ancestor of the other and so FOP's model of the FO elements >> >> should reflect this, I believe. >> >> >> >> On Fri, Sep 30, 2011 at 8:43 AM, Glenn Adams wrote: >> >> > if I recall, I need this inheritance (from Inline) to work in the >> >> > complex >> >> > script branch as well >> >> > >> >> > On Fri, Sep 30, 2011 at 3:12 PM, Simon Pepping < >> spepp...@leverkruid.eu> >> >> > wrote: >> >> >> >> >> >> On Thu, Sep 29, 2011 at 10:18:54AM -, phancock@apache.orgwrote: >> >> >> > Author: phancock >> >> >> > Date: Thu Sep 29 10:18:53 2011 >> >> >> > New Revision: 1177251 >> >> >> > >> >> >> > URL: http://svn.apache.org/viewvc?rev=1177251&view=rev >> >> >> > Log: >> >> >> > Fix FO tree hierarchy: BasicLink shouldn't inherit from Inline >> >> >> >> >> >> Why? A basic-link is an inline object which generates inline areas. >> >> >> >> >> >> Simon >> >> > >> >> > >> > >> > >> > >
Re: svn commit: r1177251 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fo/flow/BasicLink.java layoutmgr/inline/InlineLayoutManager.java pdf/PDFFactory.java
let's step back a minute; what was the problem you were trying to solve? what was broken? how did your change fix it? if you made this change just because you think instanceof Inline should return false on a BasicLink, then this change would seem gratuitous it is wholly reasonable that BasicLink may share the implementation of Inline as previously held; your change required you to copy/paste existing code from Inline into BasicLink and to alter InlineLayoutManager for no purpose other than accommodating your change IMO you should revert the change On Fri, Sep 30, 2011 at 9:18 PM, Peter Hancock wrote: > By ancestor I refer to the relationship with respect to the fo: > element hierarchy: Since the definition of fo:basic-link does not > depend upon fo:inline, we cannot conclude that fo:basic-link is an > fo:inline. > > The parameter entity "%inline;" refers to inline-level fo elements, > fo:inline and fo:basic-link being members, and this is now reflected > on the FOP FO object hierarchy, where Inline and BasicLink extend > InlineLevel > > Have I understood the recommendation correctly, or have I missed anything? > > On Fri, Sep 30, 2011 at 1:18 PM, Glenn Adams wrote: > > i'm not sure what you mean by 'ancestor', since containment relation is > not > > at issue here; > > your argument is counter to the definition of the parameter entity > %inline; > > defined in XSL 1.1 Section 6.2 > > > > The parameter entity, "%inline;" in the content models below, contains > the > > following formatting objects: > > > > bidi-override > > character > > external-graphic > > instream-foreign-object > > inline > > inline-container > > leader > > page-number > > page-number-citation > > page-number-citation-last > > scaling-value-citation > > basic-link > > multi-toggle > > index-page-citation-list > > > > i believe you should first restore the previous state of affairs, and > then, > > if you wish to continue making the case that it is not inline, take it up > > with the group and get consensus before making what appears to be a > possibly > > unjustified architectural change > > > > On Fri, Sep 30, 2011 at 5:31 PM, Peter Hancock > > wrote: > >> > >> While fo:basic-link and fo:inline are both inline level elements, one > >> is not the ancestor of the other and so FOP's model of the FO elements > >> should reflect this, I believe. > >> > >> On Fri, Sep 30, 2011 at 8:43 AM, Glenn Adams wrote: > >> > if I recall, I need this inheritance (from Inline) to work in the > >> > complex > >> > script branch as well > >> > > >> > On Fri, Sep 30, 2011 at 3:12 PM, Simon Pepping < > spepp...@leverkruid.eu> > >> > wrote: > >> >> > >> >> On Thu, Sep 29, 2011 at 10:18:54AM -, phanc...@apache.org wrote: > >> >> > Author: phancock > >> >> > Date: Thu Sep 29 10:18:53 2011 > >> >> > New Revision: 1177251 > >> >> > > >> >> > URL: http://svn.apache.org/viewvc?rev=1177251&view=rev > >> >> > Log: > >> >> > Fix FO tree hierarchy: BasicLink shouldn't inherit from Inline > >> >> > >> >> Why? A basic-link is an inline object which generates inline areas. > >> >> > >> >> Simon > >> > > >> > > > > > >
Re: svn commit: r1177251 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fo/flow/BasicLink.java layoutmgr/inline/InlineLayoutManager.java pdf/PDFFactory.java
By ancestor I refer to the relationship with respect to the fo: element hierarchy: Since the definition of fo:basic-link does not depend upon fo:inline, we cannot conclude that fo:basic-link is an fo:inline. The parameter entity "%inline;" refers to inline-level fo elements, fo:inline and fo:basic-link being members, and this is now reflected on the FOP FO object hierarchy, where Inline and BasicLink extend InlineLevel Have I understood the recommendation correctly, or have I missed anything? On Fri, Sep 30, 2011 at 1:18 PM, Glenn Adams wrote: > i'm not sure what you mean by 'ancestor', since containment relation is not > at issue here; > your argument is counter to the definition of the parameter entity %inline; > defined in XSL 1.1 Section 6.2 > > The parameter entity, "%inline;" in the content models below, contains the > following formatting objects: > > bidi-override > character > external-graphic > instream-foreign-object > inline > inline-container > leader > page-number > page-number-citation > page-number-citation-last > scaling-value-citation > basic-link > multi-toggle > index-page-citation-list > > i believe you should first restore the previous state of affairs, and then, > if you wish to continue making the case that it is not inline, take it up > with the group and get consensus before making what appears to be a possibly > unjustified architectural change > > On Fri, Sep 30, 2011 at 5:31 PM, Peter Hancock > wrote: >> >> While fo:basic-link and fo:inline are both inline level elements, one >> is not the ancestor of the other and so FOP's model of the FO elements >> should reflect this, I believe. >> >> On Fri, Sep 30, 2011 at 8:43 AM, Glenn Adams wrote: >> > if I recall, I need this inheritance (from Inline) to work in the >> > complex >> > script branch as well >> > >> > On Fri, Sep 30, 2011 at 3:12 PM, Simon Pepping >> > wrote: >> >> >> >> On Thu, Sep 29, 2011 at 10:18:54AM -, phanc...@apache.org wrote: >> >> > Author: phancock >> >> > Date: Thu Sep 29 10:18:53 2011 >> >> > New Revision: 1177251 >> >> > >> >> > URL: http://svn.apache.org/viewvc?rev=1177251&view=rev >> >> > Log: >> >> > Fix FO tree hierarchy: BasicLink shouldn't inherit from Inline >> >> >> >> Why? A basic-link is an inline object which generates inline areas. >> >> >> >> Simon >> > >> > > >
Re: svn commit: r1177251 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fo/flow/BasicLink.java layoutmgr/inline/InlineLayoutManager.java pdf/PDFFactory.java
i'm not sure what you mean by 'ancestor', since containment relation is not at issue here; your argument is counter to the definition of the parameter entity %inline; defined in XSL 1.1 Section 6.2 The parameter entity, "%inline;" in the content models below, contains the following formatting objects: bidi-override character external-graphic instream-foreign-object inline inline-container leader page-number page-number-citation page-number-citation-last scaling-value-citation basic-link multi-toggle index-page-citation-list i believe you should first restore the previous state of affairs, and then, if you wish to continue making the case that it is not inline, take it up with the group and get consensus before making what appears to be a possibly unjustified architectural change On Fri, Sep 30, 2011 at 5:31 PM, Peter Hancock wrote: > While fo:basic-link and fo:inline are both inline level elements, one > is not the ancestor of the other and so FOP's model of the FO elements > should reflect this, I believe. > > On Fri, Sep 30, 2011 at 8:43 AM, Glenn Adams wrote: > > if I recall, I need this inheritance (from Inline) to work in the complex > > script branch as well > > > > On Fri, Sep 30, 2011 at 3:12 PM, Simon Pepping > > wrote: > >> > >> On Thu, Sep 29, 2011 at 10:18:54AM -, phanc...@apache.org wrote: > >> > Author: phancock > >> > Date: Thu Sep 29 10:18:53 2011 > >> > New Revision: 1177251 > >> > > >> > URL: http://svn.apache.org/viewvc?rev=1177251&view=rev > >> > Log: > >> > Fix FO tree hierarchy: BasicLink shouldn't inherit from Inline > >> > >> Why? A basic-link is an inline object which generates inline areas. > >> > >> Simon > > > > >
Re: svn commit: r1177251 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fo/flow/BasicLink.java layoutmgr/inline/InlineLayoutManager.java pdf/PDFFactory.java
While fo:basic-link and fo:inline are both inline level elements, one is not the ancestor of the other and so FOP's model of the FO elements should reflect this, I believe. On Fri, Sep 30, 2011 at 8:43 AM, Glenn Adams wrote: > if I recall, I need this inheritance (from Inline) to work in the complex > script branch as well > > On Fri, Sep 30, 2011 at 3:12 PM, Simon Pepping > wrote: >> >> On Thu, Sep 29, 2011 at 10:18:54AM -, phanc...@apache.org wrote: >> > Author: phancock >> > Date: Thu Sep 29 10:18:53 2011 >> > New Revision: 1177251 >> > >> > URL: http://svn.apache.org/viewvc?rev=1177251&view=rev >> > Log: >> > Fix FO tree hierarchy: BasicLink shouldn't inherit from Inline >> >> Why? A basic-link is an inline object which generates inline areas. >> >> Simon > >
Re: svn commit: r1177251 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fo/flow/BasicLink.java layoutmgr/inline/InlineLayoutManager.java pdf/PDFFactory.java
if I recall, I need this inheritance (from Inline) to work in the complex script branch as well On Fri, Sep 30, 2011 at 3:12 PM, Simon Pepping wrote: > On Thu, Sep 29, 2011 at 10:18:54AM -, phanc...@apache.org wrote: > > Author: phancock > > Date: Thu Sep 29 10:18:53 2011 > > New Revision: 1177251 > > > > URL: http://svn.apache.org/viewvc?rev=1177251&view=rev > > Log: > > Fix FO tree hierarchy: BasicLink shouldn't inherit from Inline > > Why? A basic-link is an inline object which generates inline areas. > > Simon >
Re: svn commit: r1177251 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fo/flow/BasicLink.java layoutmgr/inline/InlineLayoutManager.java pdf/PDFFactory.java
On Thu, Sep 29, 2011 at 10:18:54AM -, phanc...@apache.org wrote: > Author: phancock > Date: Thu Sep 29 10:18:53 2011 > New Revision: 1177251 > > URL: http://svn.apache.org/viewvc?rev=1177251&view=rev > Log: > Fix FO tree hierarchy: BasicLink shouldn't inherit from Inline Why? A basic-link is an inline object which generates inline areas. Simon