Re: fix patch for bug 8795

Hi Vidhoon,

I tested your patch. Unfortunately, it does not seem to fix the bug. I
still get an extra line with zero width runs (FP_FMTMARK and
FPRUN_ENDOFPARAGRAPH). You should look into modifying the
bRunIsNonBlank test.

Regards,

Simon

On Sun, Mar 10, 2013 at 6:44 PM, Vidh <[email protected]> wrote:
> Hi all,
>
> I am Vidhoon, a newbie developer at Abiword. :)
> I am looking forward to participate in GSOC 2013.
>
> I worked on bug 8795:
> http://bugzilla.abisource.com/show_bug.cgi?id=8795
>
> I have created a first draft fix patch for the same with some detailed comments.
> I am also copy pasting the patch for easier reference at the end of
> this mail body.
>
> Please provide your valuable suggestions so that I can improve this
> patch and get it merged.
> I could not find any template mails in mailing list archives to send bug fixes.
> So excuse me if I am not following any procedures. Please indicate the
> same and I will oblige.
>
> Pointers to coding conventions (some I was able to note from existing
> code and pick automatically) are also welcome to improve on this first
> draft patch.
>
> Index: text/fmt/xp/fb_LineBreaker.cpp
> ===================================================================
> --- text/fmt/xp/fb_LineBreaker.cpp      (revision 32687)
> +++ text/fmt/xp/fb_LineBreaker.cpp      (working copy)
> @@ -37,7 +37,8 @@
>         m_pFirstRunToKeep(NULL),
>         m_pLastRunToKeep(NULL),
>         m_iMaxLineWidth(0),
> -       m_iWorkingLineWidth(0)
> +       m_iWorkingLineWidth(0),
> +       m_iTrailingSpace(-1)
>  {
>         xxx_UT_DEBUGMSG(("fb_LineBreaker %x created \n",this));
>  }
> @@ -630,7 +631,7 @@
>                         UT_ASSERT(pNewLine);    // TODO check for outofmem
>                         pNextLine = pNewLine;
>                         m_iMaxLineWidth = pNextLine->getMaxWidth();
> -                       xxx_UT_DEBUGMSG(("!!!! Generated a new Line \n"));
> +                       xxx_UT_DEBUGMSG(("!!!! Generated a new Line \n"));
>                 }
>                 else
>                 {
> @@ -648,7 +649,43 @@
>                 while (pRunToBump && pLine->getNumRunsInLine() &&
> (pLine->getLastRun() != m_pLastRunToKeep))
>                 {
>                         UT_ASSERT(pRunToBump->getLine() == pLine);
> -                       xxx_UT_DEBUGMSG(("RunToBump %x Type %d Offset %d Length %d
> \n",pRunToBump,pRunToBump->getType(),pRunToBump->getBlockOffset(),pRunToBump->getLength()));
> +                       if(pRunToBump->getType() == FPRUN_ENDOFPARAGRAPH )
> +                       {
> +                               xxx_UT_DEBUGMSG(("Trailing space present in prev run
> %d\n",pRunToBump->getPrevRun()->findTrailingSpaceDistance()));
> +/*
> +FIX for bug 8795
> +================
> +if EOP then
> +       if prev run has trailing white space then
> +               1)store actual trailing space
> +                2)delay run bump till trailing space is exhausted
> +        else
> +                bump contents into newline
> +*/
> +
> +
> if(pRunToBump->getPrevRun()->findTrailingSpaceDistance())
> +                               {
> +                                       if(m_iTrailingSpace == -1)
> +                                       {
> +                                               xxx_UT_DEBUGMSG(("backing up trailing space"));
> +                                               m_iTrailingSpace = pRunToBump->getPrevRun()->findTrailingSpaceDistance();
> +                                       }
> +                                       if(pLine->getFilledWidth() - m_iTrailingSpace < m_iMaxLineWidth)
> +                                       {
> +                                               xxx_UT_DEBUGMSG(("Filledwidth %d TrailingSpace %d FW-TS %d
> MaxWidth %d",pLine->getFilledWidth(),m_iTrailingSpace,pLine->getFilledWidth()
> - m_iTrailingSpace, m_iMaxLineWidth));
> +                                               break;
> +                                        }
> +                                       else
> +                                       {
> +                                               xxx_UT_DEBUGMSG(("trailing space over.. should now start newline"));
> +                                                m_iTrailingSpace = -1;
> +                                       }
> +                                }
> +
> +
> +                       }
> +
> +                       UT_DEBUGMSG(("RunToBump %x Type %d Offset %d Length %d
> \n",pRunToBump,pRunToBump->getType(),pRunToBump->getBlockOffset(),pRunToBump->getLength()));
>                         if(!pLine->removeRun(pRunToBump))
>                         {
>  //
> @@ -673,8 +710,12 @@
>                         xxx_UT_DEBUGMSG(("Next runToBump %x \n",pRunToBump));
>                 }
>         }
> -
> -       UT_ASSERT((!m_pLastRunToKeep) || (pLine->getLastRun() == m_pLastRunToKeep));
> +/*
> +below assert not applicable now since when trailing space
> +is present the current line is not broken immediately and
> +delayed till trailing space is exhausted by non blank characters
> +*/
> +       //UT_ASSERT((!m_pLastRunToKeep) || (pLine->getLastRun() ==
> m_pLastRunToKeep));
>  #if DEBUG
>         pLine->assertLineListIntegrity();
>         if(pNextLine)
> Index: text/fmt/xp/fb_LineBreaker.h
> ===================================================================
> --- text/fmt/xp/fb_LineBreaker.h        (revision 32687)
> +++ text/fmt/xp/fb_LineBreaker.h        (working copy)
> @@ -54,6 +54,7 @@
>
>         UT_sint32       m_iMaxLineWidth;
>         UT_sint32       m_iWorkingLineWidth;
> +       UT_sint32       m_iTrailingSpace;
>  };
>
>  #endif /* FB_LINEBREAKER_H */
>
>
> regards,
> --
> Vidhoon Viswanathan
> +91 7760711773