You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fop-commits@xmlgraphics.apache.org by vh...@apache.org on 2012/07/12 15:25:34 UTC

svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Author: vhennebert
Date: Thu Jul 12 13:25:34 2012
New Revision: 1360665

URL: http://svn.apache.org/viewvc?rev=1360665&view=rev
Log:
Bugfix: When restarting layout for the last page, discard glues and penalties at the beginning of the restarted Knuth sequence.

Modified:
    xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
    xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
    xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
    xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
    xmlgraphics/fop/trunk/status.xml
    xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
    xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java

Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java?rev=1360665&r1=1360664&r2=1360665&view=diff
==============================================================================
--- xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java (original)
+++ xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java Thu Jul 12 13:25:34 2012
@@ -492,7 +492,6 @@ public abstract class AbstractBreaker {
     protected void addAreas(PageBreakingAlgorithm alg, int startPart, int partCount,
             BlockSequence originalList, BlockSequence effectiveList) {
         LayoutContext childLC;
-        // add areas
         int startElementIndex = 0;
         int endElementIndex = 0;
         int lastBreak = -1;
@@ -550,12 +549,7 @@ public abstract class AbstractBreaker {
 
             // ignore KnuthGlue and KnuthPenalty objects
             // at the beginning of the line
-            ListIterator<KnuthElement> effectiveListIterator
-                = effectiveList.listIterator(startElementIndex);
-            while (effectiveListIterator.hasNext()
-                    && !(effectiveListIterator.next()).isBox()) {
-                startElementIndex++;
-            }
+            startElementIndex = alg.par.getFirstBoxIndex(startElementIndex);
 
             if (startElementIndex <= endElementIndex) {
                 if (log.isDebugEnabled()) {
@@ -576,7 +570,9 @@ public abstract class AbstractBreaker {
                         && p < (partCount - 1)) {
                     // count the boxes whose width is not 0
                     int boxCount = 0;
-                    effectiveListIterator = effectiveList.listIterator(startElementIndex);
+                    @SuppressWarnings("unchecked")
+                    ListIterator<KnuthElement> effectiveListIterator = effectiveList
+                            .listIterator(startElementIndex);
                     while (effectiveListIterator.nextIndex() <= endElementIndex) {
                         KnuthElement tempEl = effectiveListIterator.next();
                         if (tempEl.isBox() && tempEl.getWidth() > 0) {

Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java?rev=1360665&r1=1360664&r2=1360665&view=diff
==============================================================================
--- xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java (original)
+++ xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java Thu Jul 12 13:25:34 2012
@@ -532,14 +532,15 @@ public abstract class BreakingAlgorithm 
         // index of the first KnuthBox in the sequence, in case of non-centered
         // alignment. For centered alignment, we need to take into account preceding
         // penalties+glues used for the filler spaces
-        int firstBoxIndex = startIndex;
+        int previousPosition = startIndex;
         if (alignment != Constants.EN_CENTER) {
-            firstBoxIndex = par.getFirstBoxIndex(startIndex);
+            int firstBoxIndex = par.getFirstBoxIndex(startIndex);
+            previousPosition = (firstBoxIndex >= par.size()) ? startIndex : firstBoxIndex - 1;
         }
-        firstBoxIndex = (firstBoxIndex < 0) ? 0 : firstBoxIndex;
+        previousPosition = (previousPosition < 0) ? 0 : previousPosition;
 
         // create an active node representing the starting point
-        addNode(0, createNode(firstBoxIndex, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, null));
+        addNode(0, createNode(previousPosition, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, null));
         KnuthNode lastForced = getNode(0);
 
         if (log.isTraceEnabled()) {

Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java?rev=1360665&r1=1360664&r2=1360665&view=diff
==============================================================================
--- xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java (original)
+++ xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java Thu Jul 12 13:25:34 2012
@@ -20,6 +20,7 @@
 package org.apache.fop.layoutmgr;
 
 import java.util.ArrayList;
+import java.util.Iterator;
 import java.util.List;
 import java.util.ListIterator;
 
@@ -159,46 +160,26 @@ public abstract class KnuthSequence exte
                 : (ListElement) get(index);
     }
 
-    /** @return the position index of the first box in this sequence */
-    protected int getFirstBoxIndex() {
-        if (isEmpty()) {
-            return -1;
-        } else {
-            return getFirstBoxIndex(0);
-        }
-    }
-
     /**
-     * Get the position index of the first box in this sequence,
-     * starting at the given index. If there is no box after the
-     * passed {@code startIndex}, the starting index itself is returned.
-     * @param startIndex    the starting index for the lookup
-     * @return  the absolute position index of the next box element
+     * Returns the position index of the first box in this sequence, starting at the given
+     * index. If {@code startIndex} is outside the bounds of this sequence, it is
+     * returned.
+     *
+     * @param startIndex the index from which to start the lookup
+     * @return the index of the next box element, {@link #size()} if there is no such
+     * element, {@code startIndex} if {@code (startIndex < 0 || startIndex >= size())}
      */
     protected int getFirstBoxIndex(int startIndex) {
-        if (isEmpty() || startIndex < 0 || startIndex >= size()) {
-            return -1;
+        if (startIndex < 0 || startIndex >= size()) {
+            return startIndex;
         } else {
-            ListElement element = null;
-            int posIndex = startIndex;
-            int lastIndex = size();
-            while ( posIndex < lastIndex ) {
-                element = getElement(posIndex);
-                if ( !element.isBox() ) {
-                    posIndex++;
-                } else {
-                    break;
-                }
-            }
-            if ( posIndex != startIndex ) {
-                if ( ( element != null ) && element.isBox() ) {
-                    return posIndex - 1;
-                } else {
-                    return startIndex;
-                }
-            } else {
-                return startIndex;
+            int boxIndex = startIndex;
+            @SuppressWarnings("unchecked")
+            Iterator<ListElement> iter = listIterator(startIndex);
+            while (iter.hasNext() && !iter.next().isBox()) {
+                boxIndex++;
             }
+            return boxIndex;
         }
     }
 

Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java?rev=1360665&r1=1360664&r2=1360665&view=diff
==============================================================================
--- xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java (original)
+++ xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java Thu Jul 12 13:25:34 2012
@@ -330,7 +330,7 @@ public class PageBreaker extends Abstrac
             //Get page break from which we restart
             PageBreakPosition pbp = (PageBreakPosition)
                     alg.getPageBreaks().get(restartPoint - 1);
-            newStartPos = pbp.getLeafPos() + 1;
+            newStartPos = alg.par.getFirstBoxIndex(pbp.getLeafPos() + 1);
             //Handle page break right here to avoid any side-effects
             if (newStartPos > 0) {
                 handleBreakTrait(Constants.EN_PAGE);

Modified: xmlgraphics/fop/trunk/status.xml
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/status.xml?rev=1360665&r1=1360664&r2=1360665&view=diff
==============================================================================
--- xmlgraphics/fop/trunk/status.xml (original)
+++ xmlgraphics/fop/trunk/status.xml Thu Jul 12 13:25:34 2012
@@ -63,6 +63,10 @@
       documents. Example: the fix of marks layering will be such a case when it's done.
     -->
     <release version="FOP Trunk" date="TBD">
+      <action context="Layout" dev="VH" type="fix">
+        When restarting layout for the last page, discard glues and penalties at the beginning of 
+        the restarted Knuth sequence.
+      </action>
       <action context="Test" dev="GA" type="fix">
         Fix errors and warnings in example files. Add build.xml for documentation examples.
       </action>

Modified: xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java?rev=1360665&r1=1360664&r2=1360665&view=diff
==============================================================================
--- xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java (original)
+++ xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java Thu Jul 12 13:25:34 2012
@@ -48,9 +48,6 @@ import org.apache.fop.render.intermediat
 @RunWith(Parameterized.class)
 public class IFParserTestCase extends AbstractIFTest {
 
-    /** Set this to true to get the correspondence between test number and test file. */
-    private static final boolean DEBUG = false;
-
     /**
      * Gets the parameters for this test
      *
@@ -59,19 +56,7 @@ public class IFParserTestCase extends Ab
      */
     @Parameters
     public static Collection<File[]> getParameters() throws IOException {
-        Collection<File[]> testFiles = LayoutEngineTestUtils.getLayoutTestFiles();
-        if (DEBUG) {
-            printFiles(testFiles);
-        }
-        return testFiles;
-    }
-
-    private static void printFiles(Collection<File[]> files) {
-        int index = 0;
-        for (File[] file : files) {
-            assert file.length == 1;
-            System.out.println(String.format("%3d %s", index++, file[0]));
-        }
+        return LayoutEngineTestUtils.getLayoutTestFiles();
     }
 
     /**

Modified: xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java?rev=1360665&r1=1360664&r2=1360665&view=diff
==============================================================================
--- xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java (original)
+++ xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java Thu Jul 12 13:25:34 2012
@@ -51,6 +51,9 @@ import org.apache.commons.io.filefilter.
  */
 public final class LayoutEngineTestUtils {
 
+    /** Set this to true to get the correspondence between test number and test file. */
+    private static final boolean DEBUG = false;
+
     private LayoutEngineTestUtils() {
     }
 
@@ -157,8 +160,12 @@ public final class LayoutEngineTestUtils
         }
 
         Collection<File[]> parametersForJUnit4 = new ArrayList<File[]>();
+        int index = 0;
         for (File f : files) {
             parametersForJUnit4.add(new File[] { f });
+            if (DEBUG) {
+                System.out.println(String.format("%3d %s", index++, f));
+            }
         }
 
         return parametersForJUnit4;



---------------------------------------------------------------------
To unsubscribe, e-mail: fop-commits-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: fop-commits-help@xmlgraphics.apache.org


Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Chris Bowditch <bo...@hotmail.com>.
On 16/07/2012 14:33, Glenn Adams wrote:

Hi Glenn,

> On Mon, Jul 16, 2012 at 5:19 AM, Chris Bowditch 
> <bowditch_chris@hotmail.com <ma...@hotmail.com>> wrote:
>
>     I'm in favour of changing the policy to always open a
>     Bugzilla/Jira entry for every bug fixed in FOP. However, it won't
>     always be possible to upload resources to replicate the issue.
>     Whilst we can clean any FO Files of senstive data we can't
>     necessarily do the same for Images, Fonts or other confidential
>     resources. As long as you can accept that sometimes bugs will be
>     missing resources required to replicate the issue then I'm +1 for
>     this policy change.
>
>
> BTW, I agree with this point, and I was not asking for a BZ entry that 
> included all needed to replicate it. That has never been a requirement 
> or goal, although, when it is possible to do so without an 
> extraordinary effort, it is reasonable to do as a 2nd order task. I'm 
> just looking for BZ entries (or equivalent) for substantive 
> (non-trivial) code changes to help with tracking. I'll leave it up to 
> the committer to decide what is substantive vs trivial. Examples of 
> changes I would find trivial: fixing warnings (e.g., checkstyle, etc), 
> adding/fixing javadoc or code comments, simple  (non-extensive) code 
> refactoring, cleanup, or organization. Common sense should apply.

Thanks Glenn - I just wanted to make sure you were aware of that before 
we commit to changing the policy. I do agree with the points you've 
raised. I make the development teams here have a bug in place before 
making any code changes to FOP or otherwise so fully appreciate the 
benefits that brings around tracking. I'm not sure why that wasn't 
adopted on Apache FOP in the past, but I'm in favour of implementing it, 
so here's my +1

Thanks,

Chris



Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Glenn Adams <gl...@skynav.com>.
On Mon, Jul 16, 2012 at 5:19 AM, Chris Bowditch
<bo...@hotmail.com>wrote:

> I'm in favour of changing the policy to always open a Bugzilla/Jira entry
> for every bug fixed in FOP. However, it won't always be possible to upload
> resources to replicate the issue. Whilst we can clean any FO Files of
> senstive data we can't necessarily do the same for Images, Fonts or other
> confidential resources. As long as you can accept that sometimes bugs will
> be missing resources required to replicate the issue then I'm +1 for this
> policy change.
>

BTW, I agree with this point, and I was not asking for a BZ entry that
included all needed to replicate it. That has never been a requirement or
goal, although, when it is possible to do so without an extraordinary
effort, it is reasonable to do as a 2nd order task. I'm just looking for BZ
entries (or equivalent) for substantive (non-trivial) code changes to help
with tracking. I'll leave it up to the committer to decide what is
substantive vs trivial. Examples of changes I would find trivial: fixing
warnings (e.g., checkstyle, etc), adding/fixing javadoc or code comments,
simple  (non-extensive) code refactoring, cleanup, or organization. Common
sense should apply.

Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Chris Bowditch <bo...@hotmail.com>.
On 13/07/2012 09:05, Pascal Sancho wrote:
> Hi,

Hi Pascal,

>
> IMHO, using a bugtracker system to list bugfix or changes is a good idea.
> But today, the practice in FOP project is to fill this list directly
> in the cited page.
> We can adopt a new policy here and have a change/bugfix list using
> bugtracker facilities.
> That could be done when we'll migrate to Jira.
> IIUC, Bugzilla entries will be migrated to Jira in the future.
> Waiting this migration, we can today begin to systematically fill a
> new Bugzilla entry, to keep trace in Jira.

I'm in favour of changing the policy to always open a Bugzilla/Jira 
entry for every bug fixed in FOP. However, it won't always be possible 
to upload resources to replicate the issue. Whilst we can clean any FO 
Files of senstive data we can't necessarily do the same for Images, 
Fonts or other confidential resources. As long as you can accept that 
sometimes bugs will be missing resources required to replicate the issue 
then I'm +1 for this policy change.

Thanks,

Chris

>
> WDYT?
>
>
> 2012/7/12 Glenn Adams <gl...@skynav.com>:
>> On Thu, Jul 12, 2012 at 10:47 AM, Vincent Hennebert <vh...@gmail.com>
>> wrote:
>>> On 12/07/12 16:17, Glenn Adams wrote:
>>>> If there is no bug entry in bugzilla, then there is/was no bug. Since
>>>> this
>>>> is clearly a bug fix, there should be a documentation trail through the
>>>> bug
>>>> database. So please create an entry and do so in the future. The
>>>> status.xml
>>>> document is only an informal paraphrase of bug database entries, and
>>>> should
>>>> not be considered the authoritative list of bugs.
>>> Well it /is/ authoritative, in the sense that its content is used to
>>> display the list of changes on the website:
>>> http://xmlgraphics.apache.org/fop/changes.html
>>
>> The fact that status.xml makes reference to bugzilla entries, and not the
>> other way around shows the latter is more authoritative than the former.
>>
>>>
>>>
>>> As long as it’s the case, duplicating entries in Bugzilla is just
>>> a waste of time.
>>
>> Not it isn't. It is good time spent to document the work on FOP/XGC, etc.
>> Others are doing this, so you should follow suit and not be remiss in your
>> duties.
>>
>>>
>>> Once that status.xml has been deprecated and an other mechanism
>>> implemented to extract the list of changes from Bugzilla and display
>>> them on the website, I’ll certainly start creating entries in Bugzilla.
>>> In the meantime, I don’t see the point of doing both.
>>>
>>> Unless, again, I’m missing something.
>>
>> It's a matter of following best current practice. Failing to record in BZ is
>> not following BCP.
>>
>
>



Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Glenn Adams <gl...@skynav.com>.
On Fri, Jul 13, 2012 at 9:08 AM, Vincent Hennebert <vh...@gmail.com>wrote:

> We can decide to change that and use a bug tracking system instead, but
> in the same time we do that we will have to have in place a new
> mechanism to generate the changes [1] page. Also, we will most likely
> have to find a way to feed the BTS with the content of status.xml that
> is not there yet.
>

The two issues are not necessarily connected: we don't need to wait until
we are generating status/changes from the BTS to make entries in BZ when we
make fixes. We can do the latter now without any other action. That's all
I'm asking be done now. The other can wait, perhaps best done after
infrastructure has transitioned us to JIRA.

Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Chris Bowditch <bo...@hotmail.com>.
On 18/07/2012 15:24, Clay Leeds wrote:

Hi Clay,
> OT, but I've made some progress on the CMS. I now have dynamic content working (separate header and sidenav for FOP, Batik & XGC).

Thanks for the update. That is good news.

Chris

>
> More relevant comments below...
>
> On Jul 18, 2012, at 5:17 AM, Vincent Hennebert <vh...@gmail.com> wrote:
>
>> Hi Alexios,
>>
>> thanks for you input.
>>
>> On 18/07/12 01:05, Alexios Giotis wrote:
>>> Hi Vincent,
>>>
>>> On Jul 17, 2012, at 12:10 PM, Vincent Hennebert wrote:
>>>
>>> It improves documentation. For example, I frequently execute "svn annotate", read the usually brief commit message and then refer to the bug for more info (and context in my case). I don't like the fact that I have to rely to another system, but it helps, especially for old change sets.
>> Well, the problem is probably not the lack of a BTS here, it’s probably
>> the commit message that shouldn’t be that short. And a longer
>> description should be in status.xml anyway. Also, I find the list of
>> comments that usually appears in Bugzilla entries confusing more than
>> anything else. You have to wander through the comments to understand
>> what is going on.
> Perhaps, if something significant changes, the OP or someone for a bug can update the description (e. g., w 'UPDATE:') where they pick out the meat of a discussion and stik it in the Description field.
>
>> That said, if a bug affects the rendering part of FOP which is not
>> really unit-testable at the moment, the commit is unlikely to contain
>> any test, so it helps to be able to retrieve an example output on
>> Bugzilla. And I suppose that for the sake of consistency, the same
>> should be done for layout bugs.
> I would expect an example attachment of PDFs or Afp, Ps, etc. for layout bugs.
>
>> Also, I’m just remembering that we will at some point switch to the
>> Apache CMS to generate the website and that the status.xml logic may not
>> be ported there. That’s probably the biggest factor that would push me
>> to switch to Bugzilla actually.
> My hope is that the BTS system can output a file for us. I understand that HTML files can be inserted, unchanged by the CMS. I plan on something like this for Compliance.html.
>
>> <snip/>
> Clay
>



Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Clay Leeds <th...@gmail.com>.
OT, but I've made some progress on the CMS. I now have dynamic content working (separate header and sidenav for FOP, Batik & XGC). 

More relevant comments below...

On Jul 18, 2012, at 5:17 AM, Vincent Hennebert <vh...@gmail.com> wrote:

> Hi Alexios,
> 
> thanks for you input.
> 
> On 18/07/12 01:05, Alexios Giotis wrote:
>> Hi Vincent,
>> 
>> On Jul 17, 2012, at 12:10 PM, Vincent Hennebert wrote:
>> 
>> It improves documentation. For example, I frequently execute "svn annotate", read the usually brief commit message and then refer to the bug for more info (and context in my case). I don't like the fact that I have to rely to another system, but it helps, especially for old change sets.
> 
> Well, the problem is probably not the lack of a BTS here, it’s probably
> the commit message that shouldn’t be that short. And a longer
> description should be in status.xml anyway. Also, I find the list of
> comments that usually appears in Bugzilla entries confusing more than
> anything else. You have to wander through the comments to understand
> what is going on.

Perhaps, if something significant changes, the OP or someone for a bug can update the description (e. g., w 'UPDATE:') where they pick out the meat of a discussion and stik it in the Description field. 

> That said, if a bug affects the rendering part of FOP which is not
> really unit-testable at the moment, the commit is unlikely to contain
> any test, so it helps to be able to retrieve an example output on
> Bugzilla. And I suppose that for the sake of consistency, the same
> should be done for layout bugs.

I would expect an example attachment of PDFs or Afp, Ps, etc. for layout bugs. 

> Also, I’m just remembering that we will at some point switch to the
> Apache CMS to generate the website and that the status.xml logic may not
> be ported there. That’s probably the biggest factor that would push me
> to switch to Bugzilla actually.

My hope is that the BTS system can output a file for us. I understand that HTML files can be inserted, unchanged by the CMS. I plan on something like this for Compliance.html. 

> <snip/>

Clay 

Re: Switching to JIRA [was: Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/]

Posted by Glenn Adams <gl...@skynav.com>.
On Wed, Jul 18, 2012 at 7:44 AM, Vincent Hennebert <vh...@gmail.com>wrote:

> On 18/07/12 13:46, Alexios Giotis wrote:
> >
> > On Jul 18, 2012, at 3:19 PM, Vincent Hennebert wrote:
> >
> >> On 18/07/12 01:14, Glenn Adams wrote:
> >>> On Tue, Jul 17, 2012 at 6:05 PM, Alexios Giotis <alex.giotis@gmail.com
> >wrote:
> >>>
> >>>> The end result is to provide a documentation trail. I initially
> replied to
> >>>> this thread because Jira is not (yet) used in FOP and it happens to
> provide
> >>>> this feature.
> >>>
> >>>
> >>> FYI, Alex, if you aren't aware of it, the project has already voted to
> move
> >>> to JIRA, and we are awaiting the infrastructure group to perform the
> >>> conversion. [1]
> >>>
> >>> [1] https://issues.apache.org/jira/browse/INFRA-4800
> >>
> >> What will happen to the current Bugzilla instance? Will it be closed or
> >> made read-only? If it’s closed the whole purpose of putting the Bugzilla
> >> number in the commit message will be defeated.
> >>
> >>
> >> Vincent
> >
> > The existing bugzilla issues will be migrated to Jira. From what I can
> see, Glenn is following the migration steps written in [1]. Related to step
> 2 (Bugzilla data worth it?), Glenn has already answered in INFRA-4800 "(2)
> historical data should be retained".
> >
> > [1] http://wiki.apache.org/general/ApacheBugzillaToJiraMigration
>
> Thanks for the pointer. At the bottom of the page it’s written that the
> Bugzilla instance should be made read-only, which answers my question
> I guess.
>

Correct.

Re: Switching to JIRA [was: Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/]

Posted by Vincent Hennebert <vh...@gmail.com>.
On 18/07/12 13:46, Alexios Giotis wrote:
> 
> On Jul 18, 2012, at 3:19 PM, Vincent Hennebert wrote:
> 
>> On 18/07/12 01:14, Glenn Adams wrote:
>>> On Tue, Jul 17, 2012 at 6:05 PM, Alexios Giotis <al...@gmail.com>wrote:
>>>
>>>> The end result is to provide a documentation trail. I initially replied to
>>>> this thread because Jira is not (yet) used in FOP and it happens to provide
>>>> this feature.
>>>
>>>
>>> FYI, Alex, if you aren't aware of it, the project has already voted to move
>>> to JIRA, and we are awaiting the infrastructure group to perform the
>>> conversion. [1]
>>>
>>> [1] https://issues.apache.org/jira/browse/INFRA-4800
>>
>> What will happen to the current Bugzilla instance? Will it be closed or
>> made read-only? If it’s closed the whole purpose of putting the Bugzilla
>> number in the commit message will be defeated.
>>
>>
>> Vincent
> 
> The existing bugzilla issues will be migrated to Jira. From what I can see, Glenn is following the migration steps written in [1]. Related to step 2 (Bugzilla data worth it?), Glenn has already answered in INFRA-4800 "(2) historical data should be retained". 
> 
> [1] http://wiki.apache.org/general/ApacheBugzillaToJiraMigration

Thanks for the pointer. At the bottom of the page it’s written that the
Bugzilla instance should be made read-only, which answers my question
I guess.

> 
> Alexios
> 

Vincent

Re: Switching to JIRA [was: Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/]

Posted by Alexios Giotis <al...@gmail.com>.
On Jul 18, 2012, at 3:19 PM, Vincent Hennebert wrote:

> On 18/07/12 01:14, Glenn Adams wrote:
>> On Tue, Jul 17, 2012 at 6:05 PM, Alexios Giotis <al...@gmail.com>wrote:
>> 
>>> The end result is to provide a documentation trail. I initially replied to
>>> this thread because Jira is not (yet) used in FOP and it happens to provide
>>> this feature.
>> 
>> 
>> FYI, Alex, if you aren't aware of it, the project has already voted to move
>> to JIRA, and we are awaiting the infrastructure group to perform the
>> conversion. [1]
>> 
>> [1] https://issues.apache.org/jira/browse/INFRA-4800
> 
> What will happen to the current Bugzilla instance? Will it be closed or
> made read-only? If it’s closed the whole purpose of putting the Bugzilla
> number in the commit message will be defeated.
> 
> 
> Vincent

The existing bugzilla issues will be migrated to Jira. From what I can see, Glenn is following the migration steps written in [1]. Related to step 2 (Bugzilla data worth it?), Glenn has already answered in INFRA-4800 "(2) historical data should be retained". 

[1] http://wiki.apache.org/general/ApacheBugzillaToJiraMigration

Alexios

Switching to JIRA [was: Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/]

Posted by Vincent Hennebert <vh...@gmail.com>.
On 18/07/12 01:14, Glenn Adams wrote:
> On Tue, Jul 17, 2012 at 6:05 PM, Alexios Giotis <al...@gmail.com>wrote:
> 
>> The end result is to provide a documentation trail. I initially replied to
>> this thread because Jira is not (yet) used in FOP and it happens to provide
>> this feature.
> 
> 
> FYI, Alex, if you aren't aware of it, the project has already voted to move
> to JIRA, and we are awaiting the infrastructure group to perform the
> conversion. [1]
> 
> [1] https://issues.apache.org/jira/browse/INFRA-4800

What will happen to the current Bugzilla instance? Will it be closed or
made read-only? If it’s closed the whole purpose of putting the Bugzilla
number in the commit message will be defeated.


Vincent

Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Glenn Adams <gl...@skynav.com>.
On Tue, Jul 17, 2012 at 6:05 PM, Alexios Giotis <al...@gmail.com>wrote:

> The end result is to provide a documentation trail. I initially replied to
> this thread because Jira is not (yet) used in FOP and it happens to provide
> this feature.


FYI, Alex, if you aren't aware of it, the project has already voted to move
to JIRA, and we are awaiting the infrastructure group to perform the
conversion. [1]

[1] https://issues.apache.org/jira/browse/INFRA-4800

Re: SVN/Jira integration was: [Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/]

Posted by mehdi houshmand <me...@gmail.com>.
Hi Alex,

Thanks for the response, that's been very informative, I think if we
considered SVN/Jenkins integration, it'd make life a lot easier. The more
we can automate, the fewer the mistakes, that's pretty much my only
reasoning, though obviously it'd have to be investigated to come up with a
workflow proposal that everyone agrees with.

As for Jenkins/Git, I absolutely agree, we're users of both and I'm a
staunch advocate of both too. Jenkins has its little quirks (I was going to
say, I wish you could create job templates, but turns out you can [1]!!!!)
but works like a dream most the time. And Git, well, I kinda like think Git
is to SVN as Linux is to Windows, both get the job done, but once you get
to appreciate what Git does, it just makes life so much easier.

While we're on the subject, if we could use a github (or similar, though I
don't know any other) system where version control, issue/bug tracking,
wikis and forks were all under one roof... Anyways, enough with my pipe
dreams.

Thanks again,

Mehdi

[1] http://blog.cloudbees.com/2012/02/using-jenkins-templates-plugin-to.html

On 19 July 2012 17:09, Alexios Giotis <al...@gmail.com> wrote:

> Hi Mehdi,
>
> I will continue the hijacking of the commit message ..... I feel that
> opening a new thread might be disturbing and I don't know if the FOP
> community is interested. I hope I am helping a little to improve the
> processes and not spamming your inbox. I am not familiar with the Apache
> infrastructure but some years ago I updated our own development
> infrastructure (SVN / Jira / Jenkins / Artifactory / Confluence / ...) and
> are currently migrating from svn to git.
>
> On Jul 19, 2012, at 11:39 AM, mehdi houshmand wrote:
>
> > Having worked a little bit with PDFBox, they seem to have quite tight
> integration between the two such that (I think) JIRA prefixes SVN commits
> automatically with JIRA IDs (the JIRA number.) I don't know if this
> integration comes out the box or whether we need to do something to get it
> working, but it seems like a good idea.
>
> Actually JIRA has no write access to SVN and is not changing commit
> messages or anything. By looking at SVN [1] and Jira [2], I can tell that
> PDFBox committers are simply following the good practice of prefixing most
> commits with the related Jira issue. I guess that this is now going to be
> followed by FOP committers too. Although all SVN/Jira integration methods
> search the whole commit message for something looking like a Jira issue
> number, I suggest that you agree on a pattern. Two commonly used patterns
> for commit messages are:
>
> FOP-1: Commit message
> [FOP-1] Commit message
>
> We follow the 2nd as we may easily spot it, it is used by other tools
> (e.g. maven release plugin) and it's easier to track it with an SVN post
> commit hook that may reject the commit, send notification emails etc.
>
> [1] http://svn.apache.org/repos/asf/pdfbox/trunk
> [2] https://issues.apache.org/jira/browse/PDFBOX
>
> Related to the SVN / Jira integration, all the methods that I know,
> require someone with admin access to the infrastructure, and they are:
>
> ** A plugin installed to Jira. It adds an additional tab that shows the
> commits related to the Jira issue, see [3].
>
> ** Attlassian FishEye [4]. The Apache Camel project is somehow using it,
> for example scroll a little down and click the "Source" tab in [5]. Looks
> good but it seams that Attlassian is hosting the service.
>
> ** A plugin [6] installed in Jenkins (continuous integration tool) that
> adds a comment in Jira with the revision and the changed files when a new
> build has been created. Strangely [6] is currently showing a random plugin
> page each time refresh the page. If this is happening while you are reading
> this, use a google cached page.
>
> I selected and deployed the plugin for Jenkins because when the comment
> appears, the reporter of the issue knows that she could get and test an
> updated snapshot version of that project. The Apache infrastructure hosts
> Jenkins at [7]. I am a very happy Jenkins (Hudson) user and I recommend it.
> It should be very easy for everyone to set up a FOP build (needs SVN
> location, ant target and a selection of some options).
>
> [3]
> https://studio.plugins.atlassian.com/wiki/display/SVN/Subversion+JIRA+plugin
> [4] http://www.atlassian.com/software/fisheye/overview
> [5] https://issues.apache.org/jira/browse/CAMEL-4954
> [6] https://wiki.jenkins-ci.org/display/JENKINS/JIRA+Plugin
> [7] https://builds.apache.org/
>
>
>
> >
> > Also, I know Jeremias is a PDFBox committer and this applies to anyone
> else familiar with these tools, if they could chime in and give us some
> suggestions as to either useful tools and/or process best-practices. I
> should confess (as has probably been made abundantly obvious from silly
> mistakes) I despise SVN and eagerly anticipate the inevitable migration to
> a DSCVS of some type (preferably but not exclusively Git.)
>
> I have been using git for over a year and have decided to switch most
> projects to git. It seems to be the preferred distributed version control
> system at the Apache foundation as it provides mirrors of the projects [8].
> The learning curve is steeper compared to other systems but it should be OK
> with the few and experienced FOP developers.
>
> [8] http://git.apache.org/
>
> HTH,
> Alexios Giotis
>
>

Re: SVN/Jira integration was: [Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/]

Posted by Alexios Giotis <al...@gmail.com>.
Hi Mehdi,

I will continue the hijacking of the commit message ..... I feel that opening a new thread might be disturbing and I don't know if the FOP community is interested. I hope I am helping a little to improve the processes and not spamming your inbox. I am not familiar with the Apache infrastructure but some years ago I updated our own development infrastructure (SVN / Jira / Jenkins / Artifactory / Confluence / ...) and are currently migrating from svn to git.

On Jul 19, 2012, at 11:39 AM, mehdi houshmand wrote:

> Having worked a little bit with PDFBox, they seem to have quite tight integration between the two such that (I think) JIRA prefixes SVN commits automatically with JIRA IDs (the JIRA number.) I don't know if this integration comes out the box or whether we need to do something to get it working, but it seems like a good idea.

Actually JIRA has no write access to SVN and is not changing commit messages or anything. By looking at SVN [1] and Jira [2], I can tell that PDFBox committers are simply following the good practice of prefixing most commits with the related Jira issue. I guess that this is now going to be followed by FOP committers too. Although all SVN/Jira integration methods search the whole commit message for something looking like a Jira issue number, I suggest that you agree on a pattern. Two commonly used patterns for commit messages are:

FOP-1: Commit message
[FOP-1] Commit message

We follow the 2nd as we may easily spot it, it is used by other tools (e.g. maven release plugin) and it's easier to track it with an SVN post commit hook that may reject the commit, send notification emails etc.

[1] http://svn.apache.org/repos/asf/pdfbox/trunk
[2] https://issues.apache.org/jira/browse/PDFBOX

Related to the SVN / Jira integration, all the methods that I know, require someone with admin access to the infrastructure, and they are:

** A plugin installed to Jira. It adds an additional tab that shows the commits related to the Jira issue, see [3].

** Attlassian FishEye [4]. The Apache Camel project is somehow using it, for example scroll a little down and click the "Source" tab in [5]. Looks good but it seams that Attlassian is hosting the service.

** A plugin [6] installed in Jenkins (continuous integration tool) that adds a comment in Jira with the revision and the changed files when a new build has been created. Strangely [6] is currently showing a random plugin page each time refresh the page. If this is happening while you are reading this, use a google cached page.

I selected and deployed the plugin for Jenkins because when the comment appears, the reporter of the issue knows that she could get and test an updated snapshot version of that project. The Apache infrastructure hosts Jenkins at [7]. I am a very happy Jenkins (Hudson) user and I recommend it. It should be very easy for everyone to set up a FOP build (needs SVN location, ant target and a selection of some options).

[3] https://studio.plugins.atlassian.com/wiki/display/SVN/Subversion+JIRA+plugin
[4] http://www.atlassian.com/software/fisheye/overview
[5] https://issues.apache.org/jira/browse/CAMEL-4954
[6] https://wiki.jenkins-ci.org/display/JENKINS/JIRA+Plugin
[7] https://builds.apache.org/



> 
> Also, I know Jeremias is a PDFBox committer and this applies to anyone else familiar with these tools, if they could chime in and give us some suggestions as to either useful tools and/or process best-practices. I should confess (as has probably been made abundantly obvious from silly mistakes) I despise SVN and eagerly anticipate the inevitable migration to a DSCVS of some type (preferably but not exclusively Git.)

I have been using git for over a year and have decided to switch most projects to git. It seems to be the preferred distributed version control system at the Apache foundation as it provides mirrors of the projects [8]. The learning curve is steeper compared to other systems but it should be OK with the few and experienced FOP developers.

[8] http://git.apache.org/

HTH,
Alexios Giotis


Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by mehdi houshmand <me...@gmail.com>.
I know this isn't directly relevant to this tread, but something else to
consider while we're on the subject of JIRA and bug tracking is integration
between SVN and JIRA itself. Having worked a little bit with PDFBox, they
seem to have quite tight integration between the two such that (I think)
JIRA prefixes SVN commits automatically with JIRA IDs (the JIRA number.) I
don't know if this integration comes out the box or whether we need to do
something to get it working, but it seems like a good idea.

Also, I know Jeremias is a PDFBox committer and this applies to anyone else
familiar with these tools, if they could chime in and give us some
suggestions as to either useful tools and/or process best-practices. I
should confess (as has probably been made abundantly obvious from silly
mistakes) I despise SVN and eagerly anticipate the inevitable migration to
a DSCVS of some type (preferably but not exclusively Git.)

I appreciate this may seem like putting the cart before the horse since we
don't really know time-lines for JIRA migration, but it's better to get
this out in the open and iron out these process/workflow protocols well
before we migrate. Maybe we should start a new thread for this? We seem to
be hijacking a commit message...

Mehdi

On 19 July 2012 09:14, Chris Bowditch <bo...@hotmail.com> wrote:

> On 18/07/2012 14:06, mehdi houshmand wrote:
>
> Hi Mehdi,
>
>  As we've seen this morning, my ineptitude at even basic bureaucracy
>> doesn't really qualify me to show a bias to either side, but I'll give my 2
>> cents worth since I am a stakeholder in this debate:
>>
>> On 18 July 2012 13:17, Vincent Hennebert <vhennebert@gmail.com <mailto:
>> vhennebert@gmail.com>> wrote:
>>
>>     <snip/>
>>     Well, the problem is probably not the lack of a BTS here, it’s
>>     probably
>>     the commit message that shouldn’t be that short. And a longer
>>     description should be in status.xml anyway. Also, I find the list of
>>     comments that usually appears in Bugzilla entries confusing more than
>>     anything else. You have to wander through the comments to understand
>>     what is going on.
>>
>>
>> Surely having lots of comments is a good thing? It means there's been a
>> discussion about the issue and possibly some conclusion has been come to as
>> to how to solve the problem. Even if the comments don't arrive at a
>> conclusion, then surely having the discussion would better document nuances
>> surrounding any particular issue?
>>
>
> +1. I totally agree and that's one of the key benefits of using BTS over
> status.xml.
>
>
>
>> The only time this can become confusing is if there are disparities in
>> the flow of the conversation between bugzilla comments and mailing list
>> posts. This doesn't happen very often so I don't really see this as a
>> consideration we should be trying to mitigate.
>>
>
> Yes I agree that is the exception rather than the rule.
>
>
>      That said, if a bug affects the rendering part of FOP which is not
>>     really unit-testable at the moment, the commit is unlikely to contain
>>     any test, so it helps to be able to retrieve an example output on
>>     Bugzilla. And I suppose that for the sake of consistency, the same
>>     should be done for layout bugs.
>>
>>
>> Agreed! I think whatever we decide, we must be consistent if only to
>> prevent confusion. It's easier following one rule for all than it is
>> remember and adhering to caveats.
>>
>>     Since everyone seems to be in favour of switching to Bugzilla, I
>>     suppose
>>     I’ll start from now on. But I urge the proponents of this move to
>>     convert the status.xml logic as soon as possible.
>>
>>
>> Again I agree with Vincent here, that status.xml gets me every time! I
>> almost invariably forget to update it, now again, that's my bad but it does
>> seem somewhat redundant if all that information is in bugzilla/JIRA. I
>> appreciate it's used for release info, but there's got to be a better
>> solution.
>>
>> I'm happy to follow the consensus on this one and I'm glad we've come to
>> an agreement.
>>
>
> Yes I think we have consenus. I can start a formal vote if necessary, but
> I don't think it is in this instance.
>
> Thanks,
>
> Chris
>
>
>> Mehdi
>>
>
>
>

Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Chris Bowditch <bo...@hotmail.com>.
On 18/07/2012 14:06, mehdi houshmand wrote:

Hi Mehdi,

> As we've seen this morning, my ineptitude at even basic bureaucracy 
> doesn't really qualify me to show a bias to either side, but I'll give 
> my 2 cents worth since I am a stakeholder in this debate:
>
> On 18 July 2012 13:17, Vincent Hennebert <vhennebert@gmail.com 
> <ma...@gmail.com>> wrote:
>
>     <snip/>
>     Well, the problem is probably not the lack of a BTS here, it’s
>     probably
>     the commit message that shouldn’t be that short. And a longer
>     description should be in status.xml anyway. Also, I find the list of
>     comments that usually appears in Bugzilla entries confusing more than
>     anything else. You have to wander through the comments to understand
>     what is going on.
>
>
> Surely having lots of comments is a good thing? It means there's been 
> a discussion about the issue and possibly some conclusion has been 
> come to as to how to solve the problem. Even if the comments don't 
> arrive at a conclusion, then surely having the discussion would better 
> document nuances surrounding any particular issue?

+1. I totally agree and that's one of the key benefits of using BTS over 
status.xml.

>
> The only time this can become confusing is if there are disparities in 
> the flow of the conversation between bugzilla comments and mailing 
> list posts. This doesn't happen very often so I don't really see this 
> as a consideration we should be trying to mitigate.

Yes I agree that is the exception rather than the rule.

>     That said, if a bug affects the rendering part of FOP which is not
>     really unit-testable at the moment, the commit is unlikely to contain
>     any test, so it helps to be able to retrieve an example output on
>     Bugzilla. And I suppose that for the sake of consistency, the same
>     should be done for layout bugs.
>
>
> Agreed! I think whatever we decide, we must be consistent if only to 
> prevent confusion. It's easier following one rule for all than it is 
> remember and adhering to caveats.
>
>     Since everyone seems to be in favour of switching to Bugzilla, I
>     suppose
>     I’ll start from now on. But I urge the proponents of this move to
>     convert the status.xml logic as soon as possible.
>
>
> Again I agree with Vincent here, that status.xml gets me every time! I 
> almost invariably forget to update it, now again, that's my bad but it 
> does seem somewhat redundant if all that information is in 
> bugzilla/JIRA. I appreciate it's used for release info, but there's 
> got to be a better solution.
>
> I'm happy to follow the consensus on this one and I'm glad we've come 
> to an agreement.

Yes I think we have consenus. I can start a formal vote if necessary, 
but I don't think it is in this instance.

Thanks,

Chris

>
> Mehdi



Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by mehdi houshmand <me...@gmail.com>.
As we've seen this morning, my ineptitude at even basic bureaucracy doesn't
really qualify me to show a bias to either side, but I'll give my 2 cents
worth since I am a stakeholder in this debate:

On 18 July 2012 13:17, Vincent Hennebert <vh...@gmail.com> wrote:

> <snip/>
> Well, the problem is probably not the lack of a BTS here, it’s probably
> the commit message that shouldn’t be that short. And a longer
> description should be in status.xml anyway. Also, I find the list of
> comments that usually appears in Bugzilla entries confusing more than
> anything else. You have to wander through the comments to understand
> what is going on.
>

Surely having lots of comments is a good thing? It means there's been a
discussion about the issue and possibly some conclusion has been come to as
to how to solve the problem. Even if the comments don't arrive at a
conclusion, then surely having the discussion would better document nuances
surrounding any particular issue?

The only time this can become confusing is if there are disparities in the
flow of the conversation between bugzilla comments and mailing list posts.
This doesn't happen very often so I don't really see this as a
consideration we should be trying to mitigate.


> That said, if a bug affects the rendering part of FOP which is not
> really unit-testable at the moment, the commit is unlikely to contain
> any test, so it helps to be able to retrieve an example output on
> Bugzilla. And I suppose that for the sake of consistency, the same
> should be done for layout bugs.
>

Agreed! I think whatever we decide, we must be consistent if only to
prevent confusion. It's easier following one rule for all than it is
remember and adhering to caveats.


> Since everyone seems to be in favour of switching to Bugzilla, I suppose
> I’ll start from now on. But I urge the proponents of this move to
> convert the status.xml logic as soon as possible.
>

Again I agree with Vincent here, that status.xml gets me every time! I
almost invariably forget to update it, now again, that's my bad but it does
seem somewhat redundant if all that information is in bugzilla/JIRA. I
appreciate it's used for release info, but there's got to be a better
solution.

I'm happy to follow the consensus on this one and I'm glad we've come to an
agreement.

Mehdi

Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Vincent Hennebert <vh...@gmail.com>.
Hi Alexios,

thanks for you input.

On 18/07/12 01:05, Alexios Giotis wrote:
> Hi Vincent,
> 
> On Jul 17, 2012, at 12:10 PM, Vincent Hennebert wrote:
> 
>> On 13/07/12 22:52, Alexios Giotis wrote:
>>> A change log or "release notes" can be generated by Jira [1]. For example, [2] is the release notes for Apache PDFBox 1.7. Bug or issue titles (at least) tend to be better, if it is known that the change log is generated by such a system. Glenn wrote very good points. It will help the FOP community if everybody is recording in BZ or Jira substantial changes, whether this is an existing practice or not. 
>>
>> An automatically generated change log is definitely desirable. But we
>> already have that with status.xml. What else does Bugzilla/JIRA bring?
> 
> It improves documentation. For example, I frequently execute "svn annotate", read the usually brief commit message and then refer to the bug for more info (and context in my case). I don't like the fact that I have to rely to another system, but it helps, especially for old change sets.

Well, the problem is probably not the lack of a BTS here, it’s probably
the commit message that shouldn’t be that short. And a longer
description should be in status.xml anyway. Also, I find the list of
comments that usually appears in Bugzilla entries confusing more than
anything else. You have to wander through the comments to understand
what is going on.

That said, if a bug affects the rendering part of FOP which is not
really unit-testable at the moment, the commit is unlikely to contain
any test, so it helps to be able to retrieve an example output on
Bugzilla. And I suppose that for the sake of consistency, the same
should be done for layout bugs.

Also, I’m just remembering that we will at some point switch to the
Apache CMS to generate the website and that the status.xml logic may not
be ported there. That’s probably the biggest factor that would push me
to switch to Bugzilla actually.


<snip/>

>> If the end result is to have to very same change log as what is
>> generated from status.xml right now, then I’m not sure I see the point.
> 
> The end result is to provide a documentation trail. I initially replied to this thread because Jira is not (yet) used in FOP and it happens to provide this feature.
> 
>>
>> If the community decides to move to a BTS to log changes, I won’t oppose
>> it but I will do it reluctantly. Also, I would be even more bothered to
>> have to both 1) add an entry to status.xml 2) add a BZ entry. So I would
>> suggest that we deprecate status.xml as soon as we make the move, and
>> find a way to integrate the new change list to the website.
> 
> You are right about deprecating status.xml. I hope that in the long term you will see the benefits of this practice and willingly file bugs (BZ) or create issues (Jira).

Since everyone seems to be in favour of switching to Bugzilla, I suppose
I’ll start from now on. But I urge the proponents of this move to
convert the status.xml logic as soon as possible.


Vincent

Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Alexios Giotis <al...@gmail.com>.
Hi Vincent,

On Jul 17, 2012, at 12:10 PM, Vincent Hennebert wrote:

> On 13/07/12 22:52, Alexios Giotis wrote:
>> A change log or "release notes" can be generated by Jira [1]. For example, [2] is the release notes for Apache PDFBox 1.7. Bug or issue titles (at least) tend to be better, if it is known that the change log is generated by such a system. Glenn wrote very good points. It will help the FOP community if everybody is recording in BZ or Jira substantial changes, whether this is an existing practice or not. 
> 
> An automatically generated change log is definitely desirable. But we
> already have that with status.xml. What else does Bugzilla/JIRA bring?

It improves documentation. For example, I frequently execute "svn annotate", read the usually brief commit message and then refer to the bug for more info (and context in my case). I don't like the fact that I have to rely to another system, but it helps, especially for old change sets.

> At the moment, adding an entry to status.xml is easy:
> • edit status.xml
> • commit it with the rest of the patch
> 
> Doing it in a BTS sounds much more cumbersome to me:
> • loading the appropriate page in the web browser
> • fill in all the appropriate entries for a new bug
> • upload a test file illustrating the defect
>  • click on the upload button
>  • click several times in the ‘Open File’ dialog box to find the test
>    file on our system
>  • click on upload
> • click on enter
> • ... You get the idea

Yes, filing a bug is definitely much more cumbersome than updating status.xml and nobody would like wasting the time of the few experienced developers. But I expect it will take less than 15 minutes to file a bug and that this time should be small compared to the time spend to make a substantive change. Small improvements, javadoc updates, typo fixes, renaming of variables, checkstyle / findbug fixes ...  don't deserve a new bug. Test files illustrating the defect are not mandatory and in most cases, they are committed as part of the unit tests (which are also not mandatory as well but good to have).


> 
> If the end result is to have to very same change log as what is
> generated from status.xml right now, then I’m not sure I see the point.

The end result is to provide a documentation trail. I initially replied to this thread because Jira is not (yet) used in FOP and it happens to provide this feature.

> 
> If the community decides to move to a BTS to log changes, I won’t oppose
> it but I will do it reluctantly. Also, I would be even more bothered to
> have to both 1) add an entry to status.xml 2) add a BZ entry. So I would
> suggest that we deprecate status.xml as soon as we make the move, and
> find a way to integrate the new change list to the website.

You are right about deprecating status.xml. I hope that in the long term you will see the benefits of this practice and willingly file bugs (BZ) or create issues (Jira).

Alexios

> 
> 
> Vincent
> 
> 
>> [1] https://confluence.atlassian.com/display/JIRA/Creating+Release+Notes
>> [2] https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12310760&version=12316940
>> 
>> Alexios Giotis
>> 
>> On Jul 13, 2012, at 6:08 PM, Vincent Hennebert wrote:
>> 
>>> We can decide to change that and use a bug tracking system instead, but
>>> in the same time we do that we will have to have in place a new
>>> mechanism to generate the changes [1] page. Also, we will most likely
>>> have to find a way to feed the BTS with the content of status.xml that
>>> is not there yet.
>>> 
>>> [1] http://xmlgraphics.apache.org/fop/changes.html
>> 
> 


Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Vincent Hennebert <vh...@gmail.com>.
On 13/07/12 22:52, Alexios Giotis wrote:
> A change log or "release notes" can be generated by Jira [1]. For example, [2] is the release notes for Apache PDFBox 1.7. Bug or issue titles (at least) tend to be better, if it is known that the change log is generated by such a system. Glenn wrote very good points. It will help the FOP community if everybody is recording in BZ or Jira substantial changes, whether this is an existing practice or not. 

An automatically generated change log is definitely desirable. But we
already have that with status.xml. What else does Bugzilla/JIRA bring?
At the moment, adding an entry to status.xml is easy:
• edit status.xml
• commit it with the rest of the patch

Doing it in a BTS sounds much more cumbersome to me:
• loading the appropriate page in the web browser
• fill in all the appropriate entries for a new bug
• upload a test file illustrating the defect
  • click on the upload button
  • click several times in the ‘Open File’ dialog box to find the test
    file on our system
  • click on upload
• click on enter
• ... You get the idea

If the end result is to have to very same change log as what is
generated from status.xml right now, then I’m not sure I see the point.

If the community decides to move to a BTS to log changes, I won’t oppose
it but I will do it reluctantly. Also, I would be even more bothered to
have to both 1) add an entry to status.xml 2) add a BZ entry. So I would
suggest that we deprecate status.xml as soon as we make the move, and
find a way to integrate the new change list to the website.


Vincent


> [1] https://confluence.atlassian.com/display/JIRA/Creating+Release+Notes
> [2] https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12310760&version=12316940
> 
> Alexios Giotis
> 
> On Jul 13, 2012, at 6:08 PM, Vincent Hennebert wrote:
> 
>> We can decide to change that and use a bug tracking system instead, but
>> in the same time we do that we will have to have in place a new
>> mechanism to generate the changes [1] page. Also, we will most likely
>> have to find a way to feed the BTS with the content of status.xml that
>> is not there yet.
>>
>> [1] http://xmlgraphics.apache.org/fop/changes.html
> 


Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Alexios Giotis <al...@gmail.com>.
A change log or "release notes" can be generated by Jira [1]. For example, [2] is the release notes for Apache PDFBox 1.7. Bug or issue titles (at least) tend to be better, if it is known that the change log is generated by such a system. Glenn wrote very good points. It will help the FOP community if everybody is recording in BZ or Jira substantial changes, whether this is an existing practice or not. 

[1] https://confluence.atlassian.com/display/JIRA/Creating+Release+Notes
[2] https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12310760&version=12316940

Alexios Giotis

On Jul 13, 2012, at 6:08 PM, Vincent Hennebert wrote:

> We can decide to change that and use a bug tracking system instead, but
> in the same time we do that we will have to have in place a new
> mechanism to generate the changes [1] page. Also, we will most likely
> have to find a way to feed the BTS with the content of status.xml that
> is not there yet.
> 
> [1] http://xmlgraphics.apache.org/fop/changes.html


Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Vincent Hennebert <vh...@gmail.com>.
On 13/07/12 14:07, Glenn Adams wrote:
> On Fri, Jul 13, 2012 at 2:05 AM, Pascal Sancho <ps...@gmail.com>wrote:
> 
>> IMHO, using a bugtracker system to list bugfix or changes is a good idea.
>> But today, the practice in FOP project is to fill this list directly
>> in the cited page.
>> We can adopt a new policy here and have a change/bugfix list using
>> bugtracker facilities.
>> That could be done when we'll migrate to Jira.
>> IIUC, Bugzilla entries will be migrated to Jira in the future.
>> Waiting this migration, we can today begin to systematically fill a
>> new Bugzilla entry, to keep trace in Jira.
>>
> 
> Thanks, yes, I agree with this, but I don't think I'm suggesting a new
> policy. We have a policy today of telling the community to report bugs in
> BZ, then we make fixes and close (or reject) those bug reports, where fixes
> change the code base and the bug is closed and a entry made in status.xml.

We’ve never had such a policy. In fact, it happened several times in the
past that a bug fix was committed straight away after it had been raised
on fop-users.

We ask users to file a Bugzilla entry only when we can’t look at the
issue immediately, in order to keep track of it.

status.xml has always been better kept up-to-date than Bugzilla. Since
it contains information that is not in Bugzilla, it is more important.

We can decide to change that and use a bug tracking system instead, but
in the same time we do that we will have to have in place a new
mechanism to generate the changes [1] page. Also, we will most likely
have to find a way to feed the BTS with the content of status.xml that
is not there yet.

[1] http://xmlgraphics.apache.org/fop/changes.html


> When we committers make changes that are essentially bug fixes (as opposed
> to minor cleanup), then we should not bypass this existing, accepted
> practice, because doing so creates unnecessary exceptions in our
> documentation and process trail. In other words, let's be consistent, and
> document bugs we fix independently in the same fashion as when those bugs
> are first reported by the community.
> 
> I think this is a reasonable process and should not be contravened for the
> sake of expediency.
> 
> G.

Vincent


Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Pascal Sancho <ps...@gmail.com>.
I'm ok with that, but all fop team should be convinced...
So debate (if that need to discuss) is open ;-)


2012/7/13 Glenn Adams <gl...@skynav.com>:
>
> On Fri, Jul 13, 2012 at 2:05 AM, Pascal Sancho <ps...@gmail.com>
> wrote:
>>
>> IMHO, using a bugtracker system to list bugfix or changes is a good idea.
>> But today, the practice in FOP project is to fill this list directly
>> in the cited page.
>> We can adopt a new policy here and have a change/bugfix list using
>> bugtracker facilities.
>> That could be done when we'll migrate to Jira.
>> IIUC, Bugzilla entries will be migrated to Jira in the future.
>> Waiting this migration, we can today begin to systematically fill a
>> new Bugzilla entry, to keep trace in Jira.
>
>
> Thanks, yes, I agree with this, but I don't think I'm suggesting a new
> policy. We have a policy today of telling the community to report bugs in
> BZ, then we make fixes and close (or reject) those bug reports, where fixes
> change the code base and the bug is closed and a entry made in status.xml.
>
> When we committers make changes that are essentially bug fixes (as opposed
> to minor cleanup), then we should not bypass this existing, accepted
> practice, because doing so creates unnecessary exceptions in our
> documentation and process trail. In other words, let's be consistent, and
> document bugs we fix independently in the same fashion as when those bugs
> are first reported by the community.
>
> I think this is a reasonable process and should not be contravened for the
> sake of expediency.
>
> G.
>
>



-- 
pascal

Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Glenn Adams <gl...@skynav.com>.
On Fri, Jul 13, 2012 at 2:05 AM, Pascal Sancho <ps...@gmail.com>wrote:

> IMHO, using a bugtracker system to list bugfix or changes is a good idea.
> But today, the practice in FOP project is to fill this list directly
> in the cited page.
> We can adopt a new policy here and have a change/bugfix list using
> bugtracker facilities.
> That could be done when we'll migrate to Jira.
> IIUC, Bugzilla entries will be migrated to Jira in the future.
> Waiting this migration, we can today begin to systematically fill a
> new Bugzilla entry, to keep trace in Jira.
>

Thanks, yes, I agree with this, but I don't think I'm suggesting a new
policy. We have a policy today of telling the community to report bugs in
BZ, then we make fixes and close (or reject) those bug reports, where fixes
change the code base and the bug is closed and a entry made in status.xml.

When we committers make changes that are essentially bug fixes (as opposed
to minor cleanup), then we should not bypass this existing, accepted
practice, because doing so creates unnecessary exceptions in our
documentation and process trail. In other words, let's be consistent, and
document bugs we fix independently in the same fashion as when those bugs
are first reported by the community.

I think this is a reasonable process and should not be contravened for the
sake of expediency.

G.

Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Pascal Sancho <ps...@gmail.com>.
Hi,

IMHO, using a bugtracker system to list bugfix or changes is a good idea.
But today, the practice in FOP project is to fill this list directly
in the cited page.
We can adopt a new policy here and have a change/bugfix list using
bugtracker facilities.
That could be done when we'll migrate to Jira.
IIUC, Bugzilla entries will be migrated to Jira in the future.
Waiting this migration, we can today begin to systematically fill a
new Bugzilla entry, to keep trace in Jira.

WDYT?


2012/7/12 Glenn Adams <gl...@skynav.com>:
>
> On Thu, Jul 12, 2012 at 10:47 AM, Vincent Hennebert <vh...@gmail.com>
> wrote:
>>
>> On 12/07/12 16:17, Glenn Adams wrote:
>> > If there is no bug entry in bugzilla, then there is/was no bug. Since
>> > this
>> > is clearly a bug fix, there should be a documentation trail through the
>> > bug
>> > database. So please create an entry and do so in the future. The
>> > status.xml
>> > document is only an informal paraphrase of bug database entries, and
>> > should
>> > not be considered the authoritative list of bugs.
>>
>> Well it /is/ authoritative, in the sense that its content is used to
>> display the list of changes on the website:
>> http://xmlgraphics.apache.org/fop/changes.html
>
>
> The fact that status.xml makes reference to bugzilla entries, and not the
> other way around shows the latter is more authoritative than the former.
>
>>
>>
>>
>> As long as it’s the case, duplicating entries in Bugzilla is just
>> a waste of time.
>
>
> Not it isn't. It is good time spent to document the work on FOP/XGC, etc.
> Others are doing this, so you should follow suit and not be remiss in your
> duties.
>
>>
>>
>> Once that status.xml has been deprecated and an other mechanism
>> implemented to extract the list of changes from Bugzilla and display
>> them on the website, I’ll certainly start creating entries in Bugzilla.
>> In the meantime, I don’t see the point of doing both.
>>
>> Unless, again, I’m missing something.
>
>
> It's a matter of following best current practice. Failing to record in BZ is
> not following BCP.
>



-- 
pascal

Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Glenn Adams <gl...@skynav.com>.
On Thu, Jul 12, 2012 at 10:47 AM, Vincent Hennebert <vh...@gmail.com>wrote:

> On 12/07/12 16:17, Glenn Adams wrote:
> > If there is no bug entry in bugzilla, then there is/was no bug. Since
> this
> > is clearly a bug fix, there should be a documentation trail through the
> bug
> > database. So please create an entry and do so in the future. The
> status.xml
> > document is only an informal paraphrase of bug database entries, and
> should
> > not be considered the authoritative list of bugs.
>
> Well it /is/ authoritative, in the sense that its content is used to
> display the list of changes on the website:
> http://xmlgraphics.apache.org/fop/changes.html


The fact that status.xml makes reference to bugzilla entries, and not the
other way around shows the latter is more authoritative than the former.


>
>
> As long as it’s the case, duplicating entries in Bugzilla is just
> a waste of time.
>

Not it isn't. It is good time spent to document the work on FOP/XGC, etc.
Others are doing this, so you should follow suit and not be remiss in your
duties.


>
> Once that status.xml has been deprecated and an other mechanism
> implemented to extract the list of changes from Bugzilla and display
> them on the website, I’ll certainly start creating entries in Bugzilla.
> In the meantime, I don’t see the point of doing both.
>
> Unless, again, I’m missing something.
>

It's a matter of following best current practice. Failing to record in BZ
is not following BCP.

Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Vincent Hennebert <vh...@gmail.com>.
On 12/07/12 16:17, Glenn Adams wrote:
> On Thu, Jul 12, 2012 at 9:00 AM, Vincent Hennebert <vh...@gmail.com>wrote:
> 
>> On 12/07/12 14:47, Glenn Adams wrote:
>>> On Thu, Jul 12, 2012 at 7:45 AM, Vincent Hennebert <vhennebert@gmail.com
>>> wrote:
>>>
>>>> On 12/07/12 14:37, Glenn Adams wrote:
>>>>> Is this addressing a specific bug? If not, then please enter a bug and
>>>>> update the status to ensure this association for documentation and
>>>> release
>>>>> notes tracking purposes.
>>>>
>>>> I added an entry in the status.xml file.
>>>>
>>>
>>> Yes, I noticed. But there is no bug #.  There should be a documented bug
>>> against every non-trivial substantive fix of this nature.
>>
>> Why? As long as the status.xml file is the authoritative list of bug
>> fixes and implemented features, adding an entry to Bugzilla is just
>> redundant paperwork I’m not keen on doing. Unless I missed something, of
>> course...
>>
> 
> If there is no bug entry in bugzilla, then there is/was no bug. Since this
> is clearly a bug fix, there should be a documentation trail through the bug
> database. So please create an entry and do so in the future. The status.xml
> document is only an informal paraphrase of bug database entries, and should
> not be considered the authoritative list of bugs.

Well it /is/ authoritative, in the sense that its content is used to
display the list of changes on the website:
http://xmlgraphics.apache.org/fop/changes.html

As long as it’s the case, duplicating entries in Bugzilla is just
a waste of time.

Once that status.xml has been deprecated and an other mechanism
implemented to extract the list of changes from Bugzilla and display
them on the website, I’ll certainly start creating entries in Bugzilla.
In the meantime, I don’t see the point of doing both.

Unless, again, I’m missing something.

Does that make sense?
Vincent

Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Glenn Adams <gl...@skynav.com>.
On Thu, Jul 12, 2012 at 9:00 AM, Vincent Hennebert <vh...@gmail.com>wrote:

> On 12/07/12 14:47, Glenn Adams wrote:
> > On Thu, Jul 12, 2012 at 7:45 AM, Vincent Hennebert <vhennebert@gmail.com
> >wrote:
> >
> >> On 12/07/12 14:37, Glenn Adams wrote:
> >>> Is this addressing a specific bug? If not, then please enter a bug and
> >>> update the status to ensure this association for documentation and
> >> release
> >>> notes tracking purposes.
> >>
> >> I added an entry in the status.xml file.
> >>
> >
> > Yes, I noticed. But there is no bug #.  There should be a documented bug
> > against every non-trivial substantive fix of this nature.
>
> Why? As long as the status.xml file is the authoritative list of bug
> fixes and implemented features, adding an entry to Bugzilla is just
> redundant paperwork I’m not keen on doing. Unless I missed something, of
> course...
>

If there is no bug entry in bugzilla, then there is/was no bug. Since this
is clearly a bug fix, there should be a documentation trail through the bug
database. So please create an entry and do so in the future. The status.xml
document is only an informal paraphrase of bug database entries, and should
not be considered the authoritative list of bugs.

Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Vincent Hennebert <vh...@gmail.com>.
On 12/07/12 14:47, Glenn Adams wrote:
> On Thu, Jul 12, 2012 at 7:45 AM, Vincent Hennebert <vh...@gmail.com>wrote:
> 
>> On 12/07/12 14:37, Glenn Adams wrote:
>>> Is this addressing a specific bug? If not, then please enter a bug and
>>> update the status to ensure this association for documentation and
>> release
>>> notes tracking purposes.
>>
>> I added an entry in the status.xml file.
>>
> 
> Yes, I noticed. But there is no bug #.  There should be a documented bug
> against every non-trivial substantive fix of this nature.

Why? As long as the status.xml file is the authoritative list of bug
fixes and implemented features, adding an entry to Bugzilla is just
redundant paperwork I’m not keen on doing. Unless I missed something, of
course...


>>> Also, I notice you left a printf in place below.  Is that 
>>> intentional? If
>>> so, then I would suggest using something other than printf.
>>
>> It’s intentional, to have the association between test index and test
>> file when investigating failing test cases.
>>
>>
>> Vincent
>>
>>
>>
>>> On Thu, Jul 12, 2012 at 7:25 AM, <vhennebert> wrote:
>>>
>>>> Author: vhennebert
>>>> Date: Thu Jul 12 13:25:34 2012
>>>> New Revision: 1360665
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1360665&view=rev
>>>> Log:
>>>> Bugfix: When restarting layout for the last page, discard glues and
>>>> penalties at the beginning of the restarted Knuth sequence.
>>>>
>>>> Modified:
>>>>
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
>>>>
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
>>>>
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
>>>>
>>>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
>>>>     xmlgraphics/fop/trunk/status.xml
>>>>
>>>>
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
>>>>
>>>>
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java
>>>>
>>>> Modified:
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
>>>> URL:
>>>>
>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>>>>
>>>>
>> ==============================================================================
>>>> ---
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
>>>> (original)
>>>> +++
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
>>>> Thu Jul 12 13:25:34 2012
>>>> @@ -492,7 +492,6 @@ public abstract class AbstractBreaker {
>>>>      protected void addAreas(PageBreakingAlgorithm alg, int startPart,
>> int
>>>> partCount,
>>>>              BlockSequence originalList, BlockSequence effectiveList) {
>>>>          LayoutContext childLC;
>>>> -        // add areas
>>>>          int startElementIndex = 0;
>>>>          int endElementIndex = 0;
>>>>          int lastBreak = -1;
>>>> @@ -550,12 +549,7 @@ public abstract class AbstractBreaker {
>>>>
>>>>              // ignore KnuthGlue and KnuthPenalty objects
>>>>              // at the beginning of the line
>>>> -            ListIterator<KnuthElement> effectiveListIterator
>>>> -                = effectiveList.listIterator(startElementIndex);
>>>> -            while (effectiveListIterator.hasNext()
>>>> -                    && !(effectiveListIterator.next()).isBox()) {
>>>> -                startElementIndex++;
>>>> -            }
>>>> +            startElementIndex =
>>>> alg.par.getFirstBoxIndex(startElementIndex);
>>>>
>>>>              if (startElementIndex <= endElementIndex) {
>>>>                  if (log.isDebugEnabled()) {
>>>> @@ -576,7 +570,9 @@ public abstract class AbstractBreaker {
>>>>                          && p < (partCount - 1)) {
>>>>                      // count the boxes whose width is not 0
>>>>                      int boxCount = 0;
>>>> -                    effectiveListIterator =
>>>> effectiveList.listIterator(startElementIndex);
>>>> +                    @SuppressWarnings("unchecked")
>>>> +                    ListIterator<KnuthElement> effectiveListIterator =
>>>> effectiveList
>>>> +                            .listIterator(startElementIndex);
>>>>                      while (effectiveListIterator.nextIndex() <=
>>>> endElementIndex) {
>>>>                          KnuthElement tempEl =
>>>> effectiveListIterator.next();
>>>>                          if (tempEl.isBox() && tempEl.getWidth() > 0) {
>>>>
>>>> Modified:
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
>>>> URL:
>>>>
>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>>>>
>>>>
>> ==============================================================================
>>>> ---
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
>>>> (original)
>>>> +++
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
>>>> Thu Jul 12 13:25:34 2012
>>>> @@ -532,14 +532,15 @@ public abstract class BreakingAlgorithm
>>>>          // index of the first KnuthBox in the sequence, in case of
>>>> non-centered
>>>>          // alignment. For centered alignment, we need to take into
>>>> account preceding
>>>>          // penalties+glues used for the filler spaces
>>>> -        int firstBoxIndex = startIndex;
>>>> +        int previousPosition = startIndex;
>>>>          if (alignment != Constants.EN_CENTER) {
>>>> -            firstBoxIndex = par.getFirstBoxIndex(startIndex);
>>>> +            int firstBoxIndex = par.getFirstBoxIndex(startIndex);
>>>> +            previousPosition = (firstBoxIndex >= par.size()) ?
>> startIndex
>>>> : firstBoxIndex - 1;
>>>>          }
>>>> -        firstBoxIndex = (firstBoxIndex < 0) ? 0 : firstBoxIndex;
>>>> +        previousPosition = (previousPosition < 0) ? 0 :
>> previousPosition;
>>>>
>>>>          // create an active node representing the starting point
>>>> -        addNode(0, createNode(firstBoxIndex, 0, 1, 0, 0, 0, 0, 0, 0, 0,
>>>> 0, null));
>>>> +        addNode(0, createNode(previousPosition, 0, 1, 0, 0, 0, 0, 0, 0,
>>>> 0, 0, null));
>>>>          KnuthNode lastForced = getNode(0);
>>>>
>>>>          if (log.isTraceEnabled()) {
>>>>
>>>> Modified:
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
>>>> URL:
>>>>
>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>>>>
>>>>
>> ==============================================================================
>>>> ---
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
>>>> (original)
>>>> +++
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
>>>> Thu Jul 12 13:25:34 2012
>>>> @@ -20,6 +20,7 @@
>>>>  package org.apache.fop.layoutmgr;
>>>>
>>>>  import java.util.ArrayList;
>>>> +import java.util.Iterator;
>>>>  import java.util.List;
>>>>  import java.util.ListIterator;
>>>>
>>>> @@ -159,46 +160,26 @@ public abstract class KnuthSequence exte
>>>>                  : (ListElement) get(index);
>>>>      }
>>>>
>>>> -    /** @return the position index of the first box in this sequence */
>>>> -    protected int getFirstBoxIndex() {
>>>> -        if (isEmpty()) {
>>>> -            return -1;
>>>> -        } else {
>>>> -            return getFirstBoxIndex(0);
>>>> -        }
>>>> -    }
>>>> -
>>>>      /**
>>>> -     * Get the position index of the first box in this sequence,
>>>> -     * starting at the given index. If there is no box after the
>>>> -     * passed {@code startIndex}, the starting index itself is
>> returned.
>>>> -     * @param startIndex    the starting index for the lookup
>>>> -     * @return  the absolute position index of the next box element
>>>> +     * Returns the position index of the first box in this sequence,
>>>> starting at the given
>>>> +     * index. If {@code startIndex} is outside the bounds of this
>>>> sequence, it is
>>>> +     * returned.
>>>> +     *
>>>> +     * @param startIndex the index from which to start the lookup
>>>> +     * @return the index of the next box element, {@link #size()} if
>>>> there is no such
>>>> +     * element, {@code startIndex} if {@code (startIndex < 0 ||
>>>> startIndex >= size())}
>>>>       */
>>>>      protected int getFirstBoxIndex(int startIndex) {
>>>> -        if (isEmpty() || startIndex < 0 || startIndex >= size()) {
>>>> -            return -1;
>>>> +        if (startIndex < 0 || startIndex >= size()) {
>>>> +            return startIndex;
>>>>          } else {
>>>> -            ListElement element = null;
>>>> -            int posIndex = startIndex;
>>>> -            int lastIndex = size();
>>>> -            while ( posIndex < lastIndex ) {
>>>> -                element = getElement(posIndex);
>>>> -                if ( !element.isBox() ) {
>>>> -                    posIndex++;
>>>> -                } else {
>>>> -                    break;
>>>> -                }
>>>> -            }
>>>> -            if ( posIndex != startIndex ) {
>>>> -                if ( ( element != null ) && element.isBox() ) {
>>>> -                    return posIndex - 1;
>>>> -                } else {
>>>> -                    return startIndex;
>>>> -                }
>>>> -            } else {
>>>> -                return startIndex;
>>>> +            int boxIndex = startIndex;
>>>> +            @SuppressWarnings("unchecked")
>>>> +            Iterator<ListElement> iter = listIterator(startIndex);
>>>> +            while (iter.hasNext() && !iter.next().isBox()) {
>>>> +                boxIndex++;
>>>>              }
>>>> +            return boxIndex;
>>>>          }
>>>>      }
>>>>
>>>>
>>>> Modified:
>>>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
>>>> URL:
>>>>
>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>>>>
>>>>
>> ==============================================================================
>>>> ---
>>>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
>>>> (original)
>>>> +++
>>>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
>>>> Thu Jul 12 13:25:34 2012
>>>> @@ -330,7 +330,7 @@ public class PageBreaker extends Abstrac
>>>>              //Get page break from which we restart
>>>>              PageBreakPosition pbp = (PageBreakPosition)
>>>>                      alg.getPageBreaks().get(restartPoint - 1);
>>>> -            newStartPos = pbp.getLeafPos() + 1;
>>>> +            newStartPos = alg.par.getFirstBoxIndex(pbp.getLeafPos() +
>> 1);
>>>>              //Handle page break right here to avoid any side-effects
>>>>              if (newStartPos > 0) {
>>>>                  handleBreakTrait(Constants.EN_PAGE);
>>>>
>>>> Modified: xmlgraphics/fop/trunk/status.xml
>>>> URL:
>>>>
>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/status.xml?rev=1360665&r1=1360664&r2=1360665&view=diff
>>>>
>>>>
>> ==============================================================================
>>>> --- xmlgraphics/fop/trunk/status.xml (original)
>>>> +++ xmlgraphics/fop/trunk/status.xml Thu Jul 12 13:25:34 2012
>>>> @@ -63,6 +63,10 @@
>>>>        documents. Example: the fix of marks layering will be such a case
>>>> when it's done.
>>>>      -->
>>>>      <release version="FOP Trunk" date="TBD">
>>>> +      <action context="Layout" dev="VH" type="fix">
>>>> +        When restarting layout for the last page, discard glues and
>>>> penalties at the beginning of
>>>> +        the restarted Knuth sequence.
>>>> +      </action>
>>>>        <action context="Test" dev="GA" type="fix">
>>>>          Fix errors and warnings in example files. Add build.xml for
>>>> documentation examples.
>>>>        </action>
>>>>
>>>> Modified:
>>>>
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
>>>> URL:
>>>>
>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>>>>
>>>>
>> ==============================================================================
>>>> ---
>>>>
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
>>>> (original)
>>>> +++
>>>>
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
>>>> Thu Jul 12 13:25:34 2012
>>>> @@ -48,9 +48,6 @@ import org.apache.fop.render.intermediat
>>>>  @RunWith(Parameterized.class)
>>>>  public class IFParserTestCase extends AbstractIFTest {
>>>>
>>>> -    /** Set this to true to get the correspondence between test number
>>>> and test file. */
>>>> -    private static final boolean DEBUG = false;
>>>> -
>>>>      /**
>>>>       * Gets the parameters for this test
>>>>       *
>>>> @@ -59,19 +56,7 @@ public class IFParserTestCase extends Ab
>>>>       */
>>>>      @Parameters
>>>>      public static Collection<File[]> getParameters() throws
>> IOException {
>>>> -        Collection<File[]> testFiles =
>>>> LayoutEngineTestUtils.getLayoutTestFiles();
>>>> -        if (DEBUG) {
>>>> -            printFiles(testFiles);
>>>> -        }
>>>> -        return testFiles;
>>>> -    }
>>>> -
>>>> -    private static void printFiles(Collection<File[]> files) {
>>>> -        int index = 0;
>>>> -        for (File[] file : files) {
>>>> -            assert file.length == 1;
>>>> -            System.out.println(String.format("%3d %s", index++,
>> file[0]));
>>>> -        }
>>>> +        return LayoutEngineTestUtils.getLayoutTestFiles();
>>>>      }
>>>>
>>>>      /**
>>>>
>>>> Modified:
>>>>
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java
>>>> URL:
>>>>
>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>>>>
>>>>
>> ==============================================================================
>>>> ---
>>>>
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java
>>>> (original)
>>>> +++
>>>>
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java
>>>> Thu Jul 12 13:25:34 2012
>>>> @@ -51,6 +51,9 @@ import org.apache.commons.io.filefilter.
>>>>   */
>>>>  public final class LayoutEngineTestUtils {
>>>>
>>>> +    /** Set this to true to get the correspondence between test number
>>>> and test file. */
>>>> +    private static final boolean DEBUG = false;
>>>> +
>>>>      private LayoutEngineTestUtils() {
>>>>      }
>>>>
>>>> @@ -157,8 +160,12 @@ public final class LayoutEngineTestUtils
>>>>          }
>>>>
>>>>          Collection<File[]> parametersForJUnit4 = new
>> ArrayList<File[]>();
>>>> +        int index = 0;
>>>>          for (File f : files) {
>>>>              parametersForJUnit4.add(new File[] { f });
>>>> +            if (DEBUG) {
>>>> +                System.out.println(String.format("%3d %s", index++,
>> f));
>>>> +            }
>>>>          }
>>>>
>>>>          return parametersForJUnit4;
>>>>
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: fop-commits-unsubscribe@xmlgraphics.apache.org
>>>> For additional commands, e-mail:
>> fop-commits-help@xmlgraphics.apache.org
>>>>
>>>>
>>>
>>
>>
> 


Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Glenn Adams <gl...@skynav.com>.
On Thu, Jul 12, 2012 at 7:45 AM, Vincent Hennebert <vh...@gmail.com>wrote:

> On 12/07/12 14:37, Glenn Adams wrote:
> > Is this addressing a specific bug? If not, then please enter a bug and
> > update the status to ensure this association for documentation and
> release
> > notes tracking purposes.
>
> I added an entry in the status.xml file.
>

Yes, I noticed. But there is no bug #.  There should be a documented bug
against every non-trivial substantive fix of this nature.



> > Also, I notice you left a printf in place below.  Is that intentional? If
> > so, then I would suggest using something other than printf.
>
> It’s intentional, to have the association between test index and test
> file when investigating failing test cases.
>
>
> Vincent
>
>
>
> > On Thu, Jul 12, 2012 at 7:25 AM, <vhennebert> wrote:
> >
> >> Author: vhennebert
> >> Date: Thu Jul 12 13:25:34 2012
> >> New Revision: 1360665
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1360665&view=rev
> >> Log:
> >> Bugfix: When restarting layout for the last page, discard glues and
> >> penalties at the beginning of the restarted Knuth sequence.
> >>
> >> Modified:
> >>
> >>
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
> >>
> >>
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
> >>
> >>
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
> >>
> >> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
> >>     xmlgraphics/fop/trunk/status.xml
> >>
> >>
> xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
> >>
> >>
> xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java
> >>
> >> Modified:
> >>
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
> >> URL:
> >>
> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java?rev=1360665&r1=1360664&r2=1360665&view=diff
> >>
> >>
> ==============================================================================
> >> ---
> >>
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
> >> (original)
> >> +++
> >>
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
> >> Thu Jul 12 13:25:34 2012
> >> @@ -492,7 +492,6 @@ public abstract class AbstractBreaker {
> >>      protected void addAreas(PageBreakingAlgorithm alg, int startPart,
> int
> >> partCount,
> >>              BlockSequence originalList, BlockSequence effectiveList) {
> >>          LayoutContext childLC;
> >> -        // add areas
> >>          int startElementIndex = 0;
> >>          int endElementIndex = 0;
> >>          int lastBreak = -1;
> >> @@ -550,12 +549,7 @@ public abstract class AbstractBreaker {
> >>
> >>              // ignore KnuthGlue and KnuthPenalty objects
> >>              // at the beginning of the line
> >> -            ListIterator<KnuthElement> effectiveListIterator
> >> -                = effectiveList.listIterator(startElementIndex);
> >> -            while (effectiveListIterator.hasNext()
> >> -                    && !(effectiveListIterator.next()).isBox()) {
> >> -                startElementIndex++;
> >> -            }
> >> +            startElementIndex =
> >> alg.par.getFirstBoxIndex(startElementIndex);
> >>
> >>              if (startElementIndex <= endElementIndex) {
> >>                  if (log.isDebugEnabled()) {
> >> @@ -576,7 +570,9 @@ public abstract class AbstractBreaker {
> >>                          && p < (partCount - 1)) {
> >>                      // count the boxes whose width is not 0
> >>                      int boxCount = 0;
> >> -                    effectiveListIterator =
> >> effectiveList.listIterator(startElementIndex);
> >> +                    @SuppressWarnings("unchecked")
> >> +                    ListIterator<KnuthElement> effectiveListIterator =
> >> effectiveList
> >> +                            .listIterator(startElementIndex);
> >>                      while (effectiveListIterator.nextIndex() <=
> >> endElementIndex) {
> >>                          KnuthElement tempEl =
> >> effectiveListIterator.next();
> >>                          if (tempEl.isBox() && tempEl.getWidth() > 0) {
> >>
> >> Modified:
> >>
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
> >> URL:
> >>
> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java?rev=1360665&r1=1360664&r2=1360665&view=diff
> >>
> >>
> ==============================================================================
> >> ---
> >>
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
> >> (original)
> >> +++
> >>
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
> >> Thu Jul 12 13:25:34 2012
> >> @@ -532,14 +532,15 @@ public abstract class BreakingAlgorithm
> >>          // index of the first KnuthBox in the sequence, in case of
> >> non-centered
> >>          // alignment. For centered alignment, we need to take into
> >> account preceding
> >>          // penalties+glues used for the filler spaces
> >> -        int firstBoxIndex = startIndex;
> >> +        int previousPosition = startIndex;
> >>          if (alignment != Constants.EN_CENTER) {
> >> -            firstBoxIndex = par.getFirstBoxIndex(startIndex);
> >> +            int firstBoxIndex = par.getFirstBoxIndex(startIndex);
> >> +            previousPosition = (firstBoxIndex >= par.size()) ?
> startIndex
> >> : firstBoxIndex - 1;
> >>          }
> >> -        firstBoxIndex = (firstBoxIndex < 0) ? 0 : firstBoxIndex;
> >> +        previousPosition = (previousPosition < 0) ? 0 :
> previousPosition;
> >>
> >>          // create an active node representing the starting point
> >> -        addNode(0, createNode(firstBoxIndex, 0, 1, 0, 0, 0, 0, 0, 0, 0,
> >> 0, null));
> >> +        addNode(0, createNode(previousPosition, 0, 1, 0, 0, 0, 0, 0, 0,
> >> 0, 0, null));
> >>          KnuthNode lastForced = getNode(0);
> >>
> >>          if (log.isTraceEnabled()) {
> >>
> >> Modified:
> >>
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
> >> URL:
> >>
> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java?rev=1360665&r1=1360664&r2=1360665&view=diff
> >>
> >>
> ==============================================================================
> >> ---
> >>
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
> >> (original)
> >> +++
> >>
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
> >> Thu Jul 12 13:25:34 2012
> >> @@ -20,6 +20,7 @@
> >>  package org.apache.fop.layoutmgr;
> >>
> >>  import java.util.ArrayList;
> >> +import java.util.Iterator;
> >>  import java.util.List;
> >>  import java.util.ListIterator;
> >>
> >> @@ -159,46 +160,26 @@ public abstract class KnuthSequence exte
> >>                  : (ListElement) get(index);
> >>      }
> >>
> >> -    /** @return the position index of the first box in this sequence */
> >> -    protected int getFirstBoxIndex() {
> >> -        if (isEmpty()) {
> >> -            return -1;
> >> -        } else {
> >> -            return getFirstBoxIndex(0);
> >> -        }
> >> -    }
> >> -
> >>      /**
> >> -     * Get the position index of the first box in this sequence,
> >> -     * starting at the given index. If there is no box after the
> >> -     * passed {@code startIndex}, the starting index itself is
> returned.
> >> -     * @param startIndex    the starting index for the lookup
> >> -     * @return  the absolute position index of the next box element
> >> +     * Returns the position index of the first box in this sequence,
> >> starting at the given
> >> +     * index. If {@code startIndex} is outside the bounds of this
> >> sequence, it is
> >> +     * returned.
> >> +     *
> >> +     * @param startIndex the index from which to start the lookup
> >> +     * @return the index of the next box element, {@link #size()} if
> >> there is no such
> >> +     * element, {@code startIndex} if {@code (startIndex < 0 ||
> >> startIndex >= size())}
> >>       */
> >>      protected int getFirstBoxIndex(int startIndex) {
> >> -        if (isEmpty() || startIndex < 0 || startIndex >= size()) {
> >> -            return -1;
> >> +        if (startIndex < 0 || startIndex >= size()) {
> >> +            return startIndex;
> >>          } else {
> >> -            ListElement element = null;
> >> -            int posIndex = startIndex;
> >> -            int lastIndex = size();
> >> -            while ( posIndex < lastIndex ) {
> >> -                element = getElement(posIndex);
> >> -                if ( !element.isBox() ) {
> >> -                    posIndex++;
> >> -                } else {
> >> -                    break;
> >> -                }
> >> -            }
> >> -            if ( posIndex != startIndex ) {
> >> -                if ( ( element != null ) && element.isBox() ) {
> >> -                    return posIndex - 1;
> >> -                } else {
> >> -                    return startIndex;
> >> -                }
> >> -            } else {
> >> -                return startIndex;
> >> +            int boxIndex = startIndex;
> >> +            @SuppressWarnings("unchecked")
> >> +            Iterator<ListElement> iter = listIterator(startIndex);
> >> +            while (iter.hasNext() && !iter.next().isBox()) {
> >> +                boxIndex++;
> >>              }
> >> +            return boxIndex;
> >>          }
> >>      }
> >>
> >>
> >> Modified:
> >> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
> >> URL:
> >>
> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java?rev=1360665&r1=1360664&r2=1360665&view=diff
> >>
> >>
> ==============================================================================
> >> ---
> >> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
> >> (original)
> >> +++
> >> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
> >> Thu Jul 12 13:25:34 2012
> >> @@ -330,7 +330,7 @@ public class PageBreaker extends Abstrac
> >>              //Get page break from which we restart
> >>              PageBreakPosition pbp = (PageBreakPosition)
> >>                      alg.getPageBreaks().get(restartPoint - 1);
> >> -            newStartPos = pbp.getLeafPos() + 1;
> >> +            newStartPos = alg.par.getFirstBoxIndex(pbp.getLeafPos() +
> 1);
> >>              //Handle page break right here to avoid any side-effects
> >>              if (newStartPos > 0) {
> >>                  handleBreakTrait(Constants.EN_PAGE);
> >>
> >> Modified: xmlgraphics/fop/trunk/status.xml
> >> URL:
> >>
> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/status.xml?rev=1360665&r1=1360664&r2=1360665&view=diff
> >>
> >>
> ==============================================================================
> >> --- xmlgraphics/fop/trunk/status.xml (original)
> >> +++ xmlgraphics/fop/trunk/status.xml Thu Jul 12 13:25:34 2012
> >> @@ -63,6 +63,10 @@
> >>        documents. Example: the fix of marks layering will be such a case
> >> when it's done.
> >>      -->
> >>      <release version="FOP Trunk" date="TBD">
> >> +      <action context="Layout" dev="VH" type="fix">
> >> +        When restarting layout for the last page, discard glues and
> >> penalties at the beginning of
> >> +        the restarted Knuth sequence.
> >> +      </action>
> >>        <action context="Test" dev="GA" type="fix">
> >>          Fix errors and warnings in example files. Add build.xml for
> >> documentation examples.
> >>        </action>
> >>
> >> Modified:
> >>
> xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
> >> URL:
> >>
> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java?rev=1360665&r1=1360664&r2=1360665&view=diff
> >>
> >>
> ==============================================================================
> >> ---
> >>
> xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
> >> (original)
> >> +++
> >>
> xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
> >> Thu Jul 12 13:25:34 2012
> >> @@ -48,9 +48,6 @@ import org.apache.fop.render.intermediat
> >>  @RunWith(Parameterized.class)
> >>  public class IFParserTestCase extends AbstractIFTest {
> >>
> >> -    /** Set this to true to get the correspondence between test number
> >> and test file. */
> >> -    private static final boolean DEBUG = false;
> >> -
> >>      /**
> >>       * Gets the parameters for this test
> >>       *
> >> @@ -59,19 +56,7 @@ public class IFParserTestCase extends Ab
> >>       */
> >>      @Parameters
> >>      public static Collection<File[]> getParameters() throws
> IOException {
> >> -        Collection<File[]> testFiles =
> >> LayoutEngineTestUtils.getLayoutTestFiles();
> >> -        if (DEBUG) {
> >> -            printFiles(testFiles);
> >> -        }
> >> -        return testFiles;
> >> -    }
> >> -
> >> -    private static void printFiles(Collection<File[]> files) {
> >> -        int index = 0;
> >> -        for (File[] file : files) {
> >> -            assert file.length == 1;
> >> -            System.out.println(String.format("%3d %s", index++,
> file[0]));
> >> -        }
> >> +        return LayoutEngineTestUtils.getLayoutTestFiles();
> >>      }
> >>
> >>      /**
> >>
> >> Modified:
> >>
> xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java
> >> URL:
> >>
> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java?rev=1360665&r1=1360664&r2=1360665&view=diff
> >>
> >>
> ==============================================================================
> >> ---
> >>
> xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java
> >> (original)
> >> +++
> >>
> xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java
> >> Thu Jul 12 13:25:34 2012
> >> @@ -51,6 +51,9 @@ import org.apache.commons.io.filefilter.
> >>   */
> >>  public final class LayoutEngineTestUtils {
> >>
> >> +    /** Set this to true to get the correspondence between test number
> >> and test file. */
> >> +    private static final boolean DEBUG = false;
> >> +
> >>      private LayoutEngineTestUtils() {
> >>      }
> >>
> >> @@ -157,8 +160,12 @@ public final class LayoutEngineTestUtils
> >>          }
> >>
> >>          Collection<File[]> parametersForJUnit4 = new
> ArrayList<File[]>();
> >> +        int index = 0;
> >>          for (File f : files) {
> >>              parametersForJUnit4.add(new File[] { f });
> >> +            if (DEBUG) {
> >> +                System.out.println(String.format("%3d %s", index++,
> f));
> >> +            }
> >>          }
> >>
> >>          return parametersForJUnit4;
> >>
> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: fop-commits-unsubscribe@xmlgraphics.apache.org
> >> For additional commands, e-mail:
> fop-commits-help@xmlgraphics.apache.org
> >>
> >>
> >
>
>

Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Vincent Hennebert <vh...@gmail.com>.
On 12/07/12 14:37, Glenn Adams wrote:
> Is this addressing a specific bug? If not, then please enter a bug and
> update the status to ensure this association for documentation and release
> notes tracking purposes.

I added an entry in the status.xml file.


> Also, I notice you left a printf in place below.  Is that intentional? If
> so, then I would suggest using something other than printf.

It’s intentional, to have the association between test index and test
file when investigating failing test cases.


Vincent



> On Thu, Jul 12, 2012 at 7:25 AM, <vhennebert> wrote:
> 
>> Author: vhennebert
>> Date: Thu Jul 12 13:25:34 2012
>> New Revision: 1360665
>>
>> URL: http://svn.apache.org/viewvc?rev=1360665&view=rev
>> Log:
>> Bugfix: When restarting layout for the last page, discard glues and
>> penalties at the beginning of the restarted Knuth sequence.
>>
>> Modified:
>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
>>     xmlgraphics/fop/trunk/status.xml
>>
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
>>
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java
>>
>> Modified:
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
>> URL:
>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>>
>> ==============================================================================
>> ---
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
>> (original)
>> +++
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
>> Thu Jul 12 13:25:34 2012
>> @@ -492,7 +492,6 @@ public abstract class AbstractBreaker {
>>      protected void addAreas(PageBreakingAlgorithm alg, int startPart, int
>> partCount,
>>              BlockSequence originalList, BlockSequence effectiveList) {
>>          LayoutContext childLC;
>> -        // add areas
>>          int startElementIndex = 0;
>>          int endElementIndex = 0;
>>          int lastBreak = -1;
>> @@ -550,12 +549,7 @@ public abstract class AbstractBreaker {
>>
>>              // ignore KnuthGlue and KnuthPenalty objects
>>              // at the beginning of the line
>> -            ListIterator<KnuthElement> effectiveListIterator
>> -                = effectiveList.listIterator(startElementIndex);
>> -            while (effectiveListIterator.hasNext()
>> -                    && !(effectiveListIterator.next()).isBox()) {
>> -                startElementIndex++;
>> -            }
>> +            startElementIndex =
>> alg.par.getFirstBoxIndex(startElementIndex);
>>
>>              if (startElementIndex <= endElementIndex) {
>>                  if (log.isDebugEnabled()) {
>> @@ -576,7 +570,9 @@ public abstract class AbstractBreaker {
>>                          && p < (partCount - 1)) {
>>                      // count the boxes whose width is not 0
>>                      int boxCount = 0;
>> -                    effectiveListIterator =
>> effectiveList.listIterator(startElementIndex);
>> +                    @SuppressWarnings("unchecked")
>> +                    ListIterator<KnuthElement> effectiveListIterator =
>> effectiveList
>> +                            .listIterator(startElementIndex);
>>                      while (effectiveListIterator.nextIndex() <=
>> endElementIndex) {
>>                          KnuthElement tempEl =
>> effectiveListIterator.next();
>>                          if (tempEl.isBox() && tempEl.getWidth() > 0) {
>>
>> Modified:
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
>> URL:
>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>>
>> ==============================================================================
>> ---
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
>> (original)
>> +++
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
>> Thu Jul 12 13:25:34 2012
>> @@ -532,14 +532,15 @@ public abstract class BreakingAlgorithm
>>          // index of the first KnuthBox in the sequence, in case of
>> non-centered
>>          // alignment. For centered alignment, we need to take into
>> account preceding
>>          // penalties+glues used for the filler spaces
>> -        int firstBoxIndex = startIndex;
>> +        int previousPosition = startIndex;
>>          if (alignment != Constants.EN_CENTER) {
>> -            firstBoxIndex = par.getFirstBoxIndex(startIndex);
>> +            int firstBoxIndex = par.getFirstBoxIndex(startIndex);
>> +            previousPosition = (firstBoxIndex >= par.size()) ? startIndex
>> : firstBoxIndex - 1;
>>          }
>> -        firstBoxIndex = (firstBoxIndex < 0) ? 0 : firstBoxIndex;
>> +        previousPosition = (previousPosition < 0) ? 0 : previousPosition;
>>
>>          // create an active node representing the starting point
>> -        addNode(0, createNode(firstBoxIndex, 0, 1, 0, 0, 0, 0, 0, 0, 0,
>> 0, null));
>> +        addNode(0, createNode(previousPosition, 0, 1, 0, 0, 0, 0, 0, 0,
>> 0, 0, null));
>>          KnuthNode lastForced = getNode(0);
>>
>>          if (log.isTraceEnabled()) {
>>
>> Modified:
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
>> URL:
>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>>
>> ==============================================================================
>> ---
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
>> (original)
>> +++
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
>> Thu Jul 12 13:25:34 2012
>> @@ -20,6 +20,7 @@
>>  package org.apache.fop.layoutmgr;
>>
>>  import java.util.ArrayList;
>> +import java.util.Iterator;
>>  import java.util.List;
>>  import java.util.ListIterator;
>>
>> @@ -159,46 +160,26 @@ public abstract class KnuthSequence exte
>>                  : (ListElement) get(index);
>>      }
>>
>> -    /** @return the position index of the first box in this sequence */
>> -    protected int getFirstBoxIndex() {
>> -        if (isEmpty()) {
>> -            return -1;
>> -        } else {
>> -            return getFirstBoxIndex(0);
>> -        }
>> -    }
>> -
>>      /**
>> -     * Get the position index of the first box in this sequence,
>> -     * starting at the given index. If there is no box after the
>> -     * passed {@code startIndex}, the starting index itself is returned.
>> -     * @param startIndex    the starting index for the lookup
>> -     * @return  the absolute position index of the next box element
>> +     * Returns the position index of the first box in this sequence,
>> starting at the given
>> +     * index. If {@code startIndex} is outside the bounds of this
>> sequence, it is
>> +     * returned.
>> +     *
>> +     * @param startIndex the index from which to start the lookup
>> +     * @return the index of the next box element, {@link #size()} if
>> there is no such
>> +     * element, {@code startIndex} if {@code (startIndex < 0 ||
>> startIndex >= size())}
>>       */
>>      protected int getFirstBoxIndex(int startIndex) {
>> -        if (isEmpty() || startIndex < 0 || startIndex >= size()) {
>> -            return -1;
>> +        if (startIndex < 0 || startIndex >= size()) {
>> +            return startIndex;
>>          } else {
>> -            ListElement element = null;
>> -            int posIndex = startIndex;
>> -            int lastIndex = size();
>> -            while ( posIndex < lastIndex ) {
>> -                element = getElement(posIndex);
>> -                if ( !element.isBox() ) {
>> -                    posIndex++;
>> -                } else {
>> -                    break;
>> -                }
>> -            }
>> -            if ( posIndex != startIndex ) {
>> -                if ( ( element != null ) && element.isBox() ) {
>> -                    return posIndex - 1;
>> -                } else {
>> -                    return startIndex;
>> -                }
>> -            } else {
>> -                return startIndex;
>> +            int boxIndex = startIndex;
>> +            @SuppressWarnings("unchecked")
>> +            Iterator<ListElement> iter = listIterator(startIndex);
>> +            while (iter.hasNext() && !iter.next().isBox()) {
>> +                boxIndex++;
>>              }
>> +            return boxIndex;
>>          }
>>      }
>>
>>
>> Modified:
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
>> URL:
>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>>
>> ==============================================================================
>> ---
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
>> (original)
>> +++
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
>> Thu Jul 12 13:25:34 2012
>> @@ -330,7 +330,7 @@ public class PageBreaker extends Abstrac
>>              //Get page break from which we restart
>>              PageBreakPosition pbp = (PageBreakPosition)
>>                      alg.getPageBreaks().get(restartPoint - 1);
>> -            newStartPos = pbp.getLeafPos() + 1;
>> +            newStartPos = alg.par.getFirstBoxIndex(pbp.getLeafPos() + 1);
>>              //Handle page break right here to avoid any side-effects
>>              if (newStartPos > 0) {
>>                  handleBreakTrait(Constants.EN_PAGE);
>>
>> Modified: xmlgraphics/fop/trunk/status.xml
>> URL:
>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/status.xml?rev=1360665&r1=1360664&r2=1360665&view=diff
>>
>> ==============================================================================
>> --- xmlgraphics/fop/trunk/status.xml (original)
>> +++ xmlgraphics/fop/trunk/status.xml Thu Jul 12 13:25:34 2012
>> @@ -63,6 +63,10 @@
>>        documents. Example: the fix of marks layering will be such a case
>> when it's done.
>>      -->
>>      <release version="FOP Trunk" date="TBD">
>> +      <action context="Layout" dev="VH" type="fix">
>> +        When restarting layout for the last page, discard glues and
>> penalties at the beginning of
>> +        the restarted Knuth sequence.
>> +      </action>
>>        <action context="Test" dev="GA" type="fix">
>>          Fix errors and warnings in example files. Add build.xml for
>> documentation examples.
>>        </action>
>>
>> Modified:
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
>> URL:
>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>>
>> ==============================================================================
>> ---
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
>> (original)
>> +++
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
>> Thu Jul 12 13:25:34 2012
>> @@ -48,9 +48,6 @@ import org.apache.fop.render.intermediat
>>  @RunWith(Parameterized.class)
>>  public class IFParserTestCase extends AbstractIFTest {
>>
>> -    /** Set this to true to get the correspondence between test number
>> and test file. */
>> -    private static final boolean DEBUG = false;
>> -
>>      /**
>>       * Gets the parameters for this test
>>       *
>> @@ -59,19 +56,7 @@ public class IFParserTestCase extends Ab
>>       */
>>      @Parameters
>>      public static Collection<File[]> getParameters() throws IOException {
>> -        Collection<File[]> testFiles =
>> LayoutEngineTestUtils.getLayoutTestFiles();
>> -        if (DEBUG) {
>> -            printFiles(testFiles);
>> -        }
>> -        return testFiles;
>> -    }
>> -
>> -    private static void printFiles(Collection<File[]> files) {
>> -        int index = 0;
>> -        for (File[] file : files) {
>> -            assert file.length == 1;
>> -            System.out.println(String.format("%3d %s", index++, file[0]));
>> -        }
>> +        return LayoutEngineTestUtils.getLayoutTestFiles();
>>      }
>>
>>      /**
>>
>> Modified:
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java
>> URL:
>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>>
>> ==============================================================================
>> ---
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java
>> (original)
>> +++
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java
>> Thu Jul 12 13:25:34 2012
>> @@ -51,6 +51,9 @@ import org.apache.commons.io.filefilter.
>>   */
>>  public final class LayoutEngineTestUtils {
>>
>> +    /** Set this to true to get the correspondence between test number
>> and test file. */
>> +    private static final boolean DEBUG = false;
>> +
>>      private LayoutEngineTestUtils() {
>>      }
>>
>> @@ -157,8 +160,12 @@ public final class LayoutEngineTestUtils
>>          }
>>
>>          Collection<File[]> parametersForJUnit4 = new ArrayList<File[]>();
>> +        int index = 0;
>>          for (File f : files) {
>>              parametersForJUnit4.add(new File[] { f });
>> +            if (DEBUG) {
>> +                System.out.println(String.format("%3d %s", index++, f));
>> +            }
>>          }
>>
>>          return parametersForJUnit4;
>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: fop-commits-unsubscribe@xmlgraphics.apache.org
>> For additional commands, e-mail: fop-commits-help@xmlgraphics.apache.org
>>
>>
> 


Re: svn commit: r1360665 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/java/org/apache/fop/intermediate/ test/java/org/apache/fop/layoutengine/

Posted by Glenn Adams <gl...@skynav.com>.
Is this addressing a specific bug? If not, then please enter a bug and
update the status to ensure this association for documentation and release
notes tracking purposes.

Also, I notice you left a printf in place below.  Is that intentional? If
so, then I would suggest using something other than printf.

On Thu, Jul 12, 2012 at 7:25 AM, <vh...@apache.org> wrote:

> Author: vhennebert
> Date: Thu Jul 12 13:25:34 2012
> New Revision: 1360665
>
> URL: http://svn.apache.org/viewvc?rev=1360665&view=rev
> Log:
> Bugfix: When restarting layout for the last page, discard glues and
> penalties at the beginning of the restarted Knuth sequence.
>
> Modified:
>
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
>
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
>
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
>
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
>     xmlgraphics/fop/trunk/status.xml
>
> xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
>
> xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java
>
> Modified:
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
> URL:
> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>
> ==============================================================================
> ---
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
> (original)
> +++
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
> Thu Jul 12 13:25:34 2012
> @@ -492,7 +492,6 @@ public abstract class AbstractBreaker {
>      protected void addAreas(PageBreakingAlgorithm alg, int startPart, int
> partCount,
>              BlockSequence originalList, BlockSequence effectiveList) {
>          LayoutContext childLC;
> -        // add areas
>          int startElementIndex = 0;
>          int endElementIndex = 0;
>          int lastBreak = -1;
> @@ -550,12 +549,7 @@ public abstract class AbstractBreaker {
>
>              // ignore KnuthGlue and KnuthPenalty objects
>              // at the beginning of the line
> -            ListIterator<KnuthElement> effectiveListIterator
> -                = effectiveList.listIterator(startElementIndex);
> -            while (effectiveListIterator.hasNext()
> -                    && !(effectiveListIterator.next()).isBox()) {
> -                startElementIndex++;
> -            }
> +            startElementIndex =
> alg.par.getFirstBoxIndex(startElementIndex);
>
>              if (startElementIndex <= endElementIndex) {
>                  if (log.isDebugEnabled()) {
> @@ -576,7 +570,9 @@ public abstract class AbstractBreaker {
>                          && p < (partCount - 1)) {
>                      // count the boxes whose width is not 0
>                      int boxCount = 0;
> -                    effectiveListIterator =
> effectiveList.listIterator(startElementIndex);
> +                    @SuppressWarnings("unchecked")
> +                    ListIterator<KnuthElement> effectiveListIterator =
> effectiveList
> +                            .listIterator(startElementIndex);
>                      while (effectiveListIterator.nextIndex() <=
> endElementIndex) {
>                          KnuthElement tempEl =
> effectiveListIterator.next();
>                          if (tempEl.isBox() && tempEl.getWidth() > 0) {
>
> Modified:
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
> URL:
> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>
> ==============================================================================
> ---
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
> (original)
> +++
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
> Thu Jul 12 13:25:34 2012
> @@ -532,14 +532,15 @@ public abstract class BreakingAlgorithm
>          // index of the first KnuthBox in the sequence, in case of
> non-centered
>          // alignment. For centered alignment, we need to take into
> account preceding
>          // penalties+glues used for the filler spaces
> -        int firstBoxIndex = startIndex;
> +        int previousPosition = startIndex;
>          if (alignment != Constants.EN_CENTER) {
> -            firstBoxIndex = par.getFirstBoxIndex(startIndex);
> +            int firstBoxIndex = par.getFirstBoxIndex(startIndex);
> +            previousPosition = (firstBoxIndex >= par.size()) ? startIndex
> : firstBoxIndex - 1;
>          }
> -        firstBoxIndex = (firstBoxIndex < 0) ? 0 : firstBoxIndex;
> +        previousPosition = (previousPosition < 0) ? 0 : previousPosition;
>
>          // create an active node representing the starting point
> -        addNode(0, createNode(firstBoxIndex, 0, 1, 0, 0, 0, 0, 0, 0, 0,
> 0, null));
> +        addNode(0, createNode(previousPosition, 0, 1, 0, 0, 0, 0, 0, 0,
> 0, 0, null));
>          KnuthNode lastForced = getNode(0);
>
>          if (log.isTraceEnabled()) {
>
> Modified:
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
> URL:
> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>
> ==============================================================================
> ---
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
> (original)
> +++
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
> Thu Jul 12 13:25:34 2012
> @@ -20,6 +20,7 @@
>  package org.apache.fop.layoutmgr;
>
>  import java.util.ArrayList;
> +import java.util.Iterator;
>  import java.util.List;
>  import java.util.ListIterator;
>
> @@ -159,46 +160,26 @@ public abstract class KnuthSequence exte
>                  : (ListElement) get(index);
>      }
>
> -    /** @return the position index of the first box in this sequence */
> -    protected int getFirstBoxIndex() {
> -        if (isEmpty()) {
> -            return -1;
> -        } else {
> -            return getFirstBoxIndex(0);
> -        }
> -    }
> -
>      /**
> -     * Get the position index of the first box in this sequence,
> -     * starting at the given index. If there is no box after the
> -     * passed {@code startIndex}, the starting index itself is returned.
> -     * @param startIndex    the starting index for the lookup
> -     * @return  the absolute position index of the next box element
> +     * Returns the position index of the first box in this sequence,
> starting at the given
> +     * index. If {@code startIndex} is outside the bounds of this
> sequence, it is
> +     * returned.
> +     *
> +     * @param startIndex the index from which to start the lookup
> +     * @return the index of the next box element, {@link #size()} if
> there is no such
> +     * element, {@code startIndex} if {@code (startIndex < 0 ||
> startIndex >= size())}
>       */
>      protected int getFirstBoxIndex(int startIndex) {
> -        if (isEmpty() || startIndex < 0 || startIndex >= size()) {
> -            return -1;
> +        if (startIndex < 0 || startIndex >= size()) {
> +            return startIndex;
>          } else {
> -            ListElement element = null;
> -            int posIndex = startIndex;
> -            int lastIndex = size();
> -            while ( posIndex < lastIndex ) {
> -                element = getElement(posIndex);
> -                if ( !element.isBox() ) {
> -                    posIndex++;
> -                } else {
> -                    break;
> -                }
> -            }
> -            if ( posIndex != startIndex ) {
> -                if ( ( element != null ) && element.isBox() ) {
> -                    return posIndex - 1;
> -                } else {
> -                    return startIndex;
> -                }
> -            } else {
> -                return startIndex;
> +            int boxIndex = startIndex;
> +            @SuppressWarnings("unchecked")
> +            Iterator<ListElement> iter = listIterator(startIndex);
> +            while (iter.hasNext() && !iter.next().isBox()) {
> +                boxIndex++;
>              }
> +            return boxIndex;
>          }
>      }
>
>
> Modified:
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
> URL:
> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>
> ==============================================================================
> ---
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
> (original)
> +++
> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
> Thu Jul 12 13:25:34 2012
> @@ -330,7 +330,7 @@ public class PageBreaker extends Abstrac
>              //Get page break from which we restart
>              PageBreakPosition pbp = (PageBreakPosition)
>                      alg.getPageBreaks().get(restartPoint - 1);
> -            newStartPos = pbp.getLeafPos() + 1;
> +            newStartPos = alg.par.getFirstBoxIndex(pbp.getLeafPos() + 1);
>              //Handle page break right here to avoid any side-effects
>              if (newStartPos > 0) {
>                  handleBreakTrait(Constants.EN_PAGE);
>
> Modified: xmlgraphics/fop/trunk/status.xml
> URL:
> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/status.xml?rev=1360665&r1=1360664&r2=1360665&view=diff
>
> ==============================================================================
> --- xmlgraphics/fop/trunk/status.xml (original)
> +++ xmlgraphics/fop/trunk/status.xml Thu Jul 12 13:25:34 2012
> @@ -63,6 +63,10 @@
>        documents. Example: the fix of marks layering will be such a case
> when it's done.
>      -->
>      <release version="FOP Trunk" date="TBD">
> +      <action context="Layout" dev="VH" type="fix">
> +        When restarting layout for the last page, discard glues and
> penalties at the beginning of
> +        the restarted Knuth sequence.
> +      </action>
>        <action context="Test" dev="GA" type="fix">
>          Fix errors and warnings in example files. Add build.xml for
> documentation examples.
>        </action>
>
> Modified:
> xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
> URL:
> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>
> ==============================================================================
> ---
> xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
> (original)
> +++
> xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
> Thu Jul 12 13:25:34 2012
> @@ -48,9 +48,6 @@ import org.apache.fop.render.intermediat
>  @RunWith(Parameterized.class)
>  public class IFParserTestCase extends AbstractIFTest {
>
> -    /** Set this to true to get the correspondence between test number
> and test file. */
> -    private static final boolean DEBUG = false;
> -
>      /**
>       * Gets the parameters for this test
>       *
> @@ -59,19 +56,7 @@ public class IFParserTestCase extends Ab
>       */
>      @Parameters
>      public static Collection<File[]> getParameters() throws IOException {
> -        Collection<File[]> testFiles =
> LayoutEngineTestUtils.getLayoutTestFiles();
> -        if (DEBUG) {
> -            printFiles(testFiles);
> -        }
> -        return testFiles;
> -    }
> -
> -    private static void printFiles(Collection<File[]> files) {
> -        int index = 0;
> -        for (File[] file : files) {
> -            assert file.length == 1;
> -            System.out.println(String.format("%3d %s", index++, file[0]));
> -        }
> +        return LayoutEngineTestUtils.getLayoutTestFiles();
>      }
>
>      /**
>
> Modified:
> xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java
> URL:
> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>
> ==============================================================================
> ---
> xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java
> (original)
> +++
> xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java
> Thu Jul 12 13:25:34 2012
> @@ -51,6 +51,9 @@ import org.apache.commons.io.filefilter.
>   */
>  public final class LayoutEngineTestUtils {
>
> +    /** Set this to true to get the correspondence between test number
> and test file. */
> +    private static final boolean DEBUG = false;
> +
>      private LayoutEngineTestUtils() {
>      }
>
> @@ -157,8 +160,12 @@ public final class LayoutEngineTestUtils
>          }
>
>          Collection<File[]> parametersForJUnit4 = new ArrayList<File[]>();
> +        int index = 0;
>          for (File f : files) {
>              parametersForJUnit4.add(new File[] { f });
> +            if (DEBUG) {
> +                System.out.println(String.format("%3d %s", index++, f));
> +            }
>          }
>
>          return parametersForJUnit4;
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: fop-commits-unsubscribe@xmlgraphics.apache.org
> For additional commands, e-mail: fop-commits-help@xmlgraphics.apache.org
>
>