1. crash when deleting multi line selection fixed

2. BreakLine() does just discard the first line (wreaking total havoc) any
   more
3. ScrollToCursor() actually scrolls to cursor
4. style change affects the first line too - and since the first time, style
   doesn't change mysteriously any more after second Clear()
5. word cursor movement even better (did I get it right this time?)


git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@2766 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

samples/richedit/wxLayout.cpp

@@ -33,14 +33,17 @@ IMPLEMENT_APP(MyApp)
 // MyFrame
 //-----------------------------------------------------------------------------
 
-   enum ids{ ID_ADD_SAMPLE = 1, ID_CLEAR, ID_PRINT,
+enum ids
+{
+    ID_ADD_SAMPLE = 1, ID_CLEAR, ID_PRINT,
     ID_PRINT_SETUP, ID_PAGE_SETUP, ID_PREVIEW, ID_PRINT_PS,
     ID_PRINT_SETUP_PS, ID_PAGE_SETUP_PS,ID_PREVIEW_PS,
     ID_WRAP, ID_NOWRAP, ID_PASTE, ID_COPY, ID_CUT,
     ID_PASTE_PRIMARY,
     ID_FIND,
     ID_WXLAYOUT_DEBUG, ID_QUIT, ID_CLICK, ID_HTML, ID_TEXT,
-             ID_TEST, ID_LINEBREAKS_TEST, ID_LONG_TEST, ID_URL_TEST };
+    ID_TEST, ID_LINEBREAKS_TEST, ID_LONG_TEST, ID_URL_TEST
+};
 
 
 IMPLEMENT_DYNAMIC_CLASS( MyFrame, wxFrame )
@@ -119,8 +122,8 @@ MyFrame::MyFrame(void) :
    m_lwin->SetMouseTracking(true);
    m_lwin->SetEditable(true);
    m_lwin->SetWrapMargin(40);
-   m_lwin->Clear(wxROMAN,16,wxNORMAL,wxNORMAL, false);
    m_lwin->SetFocus();
+   Clear();
 
    // create and set the background bitmap (this will result in a lattice)
    static const int sizeBmp = 10;

samples/richedit/wxllist.cpp

@@ -622,20 +622,25 @@ wxLayoutObjectCmd::Layout(wxDC &dc, class wxLayoutList * llist)
 
 wxLayoutLine::wxLayoutLine(wxLayoutLine *prev, wxLayoutList *llist)
 {
-   m_LineNumber = 0;
    m_Width = m_Height = 0;
    m_Length = 0;
+
    m_updateLeft = -1;
    MarkDirty(0);
+
    m_Previous = prev;
    m_Next = NULL;
+
+   m_LineNumber = 0;
    RecalculatePosition(llist);
+
    if(m_Previous)
    {
       m_LineNumber = m_Previous->GetLineNumber() + 1;
       m_Next = m_Previous->GetNextLine();
       m_Previous->m_Next = this;
    }
+
    if(m_Next)
    {
       m_Next->m_Previous = this;
@@ -736,30 +741,45 @@ wxLayoutLine::FindObject(CoordType xpos, CoordType *offset) const
 }
 
 wxLayoutObjectList::iterator
-wxLayoutLine::FindObjectScreen(wxDC &dc,
+wxLayoutLine::FindObjectScreen(wxDC &dc, wxLayoutList *llist,
                                CoordType xpos, CoordType *cxpos,
                                bool *found) const
 {
    wxASSERT(cxpos);
-   wxASSERT(cxpos);
+
+   llist->ApplyStyle(GetStyleInfo(), dc);
+
    wxLayoutObjectList::iterator i;
    CoordType x = 0, cx = 0, width;
 
    for(i = m_ObjectList.begin(); i != NULLIT; i++)
    {
-      width = (**i).GetWidth();
+      wxLayoutObject *obj = *i;
+      if ( obj->GetType() == WXLO_TYPE_CMD )
+      {
+         // this will set the correct font for the objects which follow
+         obj->Layout(dc, llist);
+      }
+
+      width = obj->GetWidth();
       if( x <= xpos && xpos <= x + width )
       {
-         *cxpos = cx + (**i).GetOffsetScreen(dc, xpos-x);
+         *cxpos = cx + obj->GetOffsetScreen(dc, xpos-x);
-         if(found) *found = true;
+
+         if ( found )
+             *found = true;
          return i;
       }
-      x += (**i).GetWidth();
+
-      cx += (**i).GetLength();
+      x += obj->GetWidth();
+      cx += obj->GetLength();
    }
+
    // behind last object:
    *cxpos = cx;
-   if(found) *found = false;
+
+   if (found)
+       *found = false;
    return m_ObjectList.tail();
 }
 
@@ -1012,10 +1032,18 @@ wxLayoutLine::DeleteLine(bool update, wxLayoutList *llist)
    if(m_Previous)
        m_Previous->m_Next = m_Next;
 
+   wxLayoutLine *next = m_Next;
+   if ( next )
+   {
+      // get the line numbers right again
+      next->MoveLines(-1);
+   }
+
    if(update)
    {
-      m_Next->MoveLines(-1);
+      if ( next )
-      m_Next->RecalculatePositions(1, llist);
+         next->RecalculatePositions(1, llist);
+
       /* We assume that if we have more than one object in the list,
          this means that we have a command object, so we need to
          update the following lines. */
@@ -1025,7 +1053,7 @@ wxLayoutLine::DeleteLine(bool update, wxLayoutList *llist)
          )
          MarkNextDirty(-1);
    }
-   wxLayoutLine *next = m_Next;
+
    delete this;
 
    llist->DecNumLines();
@@ -1367,6 +1395,8 @@ wxLayoutLine::MergeNextLine(wxLayoutList *llist)
 #endif // 0
    }
 
+   llist->DecNumLines();
+
    delete oldnext;
 }
 
@@ -1531,6 +1561,7 @@ wxLayoutList::wxLayoutList()
 wxLayoutList::~wxLayoutList()
 {
    InternalClear();
+   Empty();
    m_FirstLine->DeleteLine(false, this);
 
    wxASSERT_MSG( m_numLines == 0, "line count calculation broken" );
@@ -1555,7 +1586,6 @@ wxLayoutList::Empty(void)
 void
 wxLayoutList::InternalClear(void)
 {
-   Empty();
    m_Selection.m_selecting = false;
    m_Selection.m_valid = false;
 
@@ -1620,6 +1650,11 @@ wxLayoutList::Clear(int family, int size, int style, int weight,
    m_DefaultStyleInfo = wxLayoutStyleInfo(family, size, style, weight,
                                         underline, fg, bg);
    m_CurrentStyleInfo = m_DefaultStyleInfo;
+
+   // Empty() should be called after we set m_DefaultStyleInfo because
+   // otherwise the style info for the first line (created in Empty()) would be
+   // incorrect
+   Empty();
 }
 
 wxPoint
@@ -1787,14 +1822,51 @@ wxLayoutList::MoveCursorWord(int n, bool untilNext)
 
    CoordType moveDistance = 0;
    CoordType offset;
-   for ( wxLOiterator i = m_CursorLine->FindObject(m_CursorPos.x, &offset);
+   wxLayoutLine *lineCur = m_CursorLine;
+   for ( wxLOiterator i = lineCur->FindObject(m_CursorPos.x, &offset);
          n != 0;
          n > 0 ? i++ : i-- )
    {
       if ( i == NULLIT )
+      {
+         if ( n > 0 )
+         {
+            // moving forward, pass to the first object of the next line
+            moveDistance++;
+            lineCur = lineCur->GetNextLine();
+            if ( lineCur )
+               i = lineCur->GetFirstObject();
+         }
+         else
+         {
+            // moving backwards, pass to the last object of the prev line
+            moveDistance--;
+            lineCur = lineCur->GetPreviousLine();
+            if ( lineCur )
+               i = lineCur->GetLastObject();
+         }
+
+         if ( i == NULLIT )
+         {
+            // moved to the end/beginning of text
             return false;
+         }
+      }
 
       wxLayoutObject *obj = *i;
+
+      if ( offset == -1 )
+      {
+         // calculate offset: we are either at the very beginning or the very
+         // end of the object, so it isn't very difficult (the only time when
+         // offset is != -1 is for the very first iteration when its value is
+         // returned by FindObject)
+         if ( n > 0 )
+            offset = 0;
+         else
+            offset = obj->GetLength();
+      }
+
       if( obj->GetType() != WXLO_TYPE_TEXT )
       {
          // any visible non text objects count as one word
@@ -1817,9 +1889,11 @@ wxLayoutList::MoveCursorWord(int n, bool untilNext)
             if ( n > 0 )
             {
                // can't move further in this text object
-               n--;
-
                canAdvance = false;
+
+               // still should move over the object border
+               moveDistance++;
+               n--;
             }
             else if ( offset > 0 )
             {
@@ -1871,14 +1945,26 @@ wxLayoutList::MoveCursorWord(int n, bool untilNext)
                }
             }
 
+            CoordType moveDelta = p - start - offset;
+            if ( (n < 0) && (offset == tobj->GetLength() - 1) )
+            {
+               // because we substracted 1 from offset in this case above, now
+               // compensate for it
+               moveDelta--;
+            }
+
+            if ( moveDelta != 0 )
+            {
+               moveDistance += moveDelta;
+
                n > 0 ? n-- : n++;
-
+            }
-            moveDistance = p - start - offset;
          }
       }
 
-      // except for the first iteration, offset is 0
+      // except for the first iteration, offset is calculated in the beginning
-      offset = 0;
+      // of the loop
+      offset = -1;
    }
 
    MoveCursorHorizontally(moveDistance);
@@ -1952,7 +2038,6 @@ bool
 wxLayoutList::LineBreak(void)
 {
    wxASSERT(m_CursorLine);
-   bool setFirst = (m_CursorLine == m_FirstLine && m_CursorPos.x == 0);
 
    AddCursorPosToUpdateRect();
 
@@ -1963,14 +2048,9 @@ wxLayoutList::LineBreak(void)
       height = m_CursorLine->GetHeight();
 
    m_CursorLine = m_CursorLine->Break(m_CursorPos.x, this);
-   if(setFirst) // we were at beginning of first line
-      m_FirstLine = m_CursorLine;
-   wxASSERT(m_FirstLine);
-   if(m_CursorPos.x != 0)
    m_CursorPos.y++;
    m_CursorPos.x = 0;
 
-
    // The following code will produce a height which is guaranteed to
    // be too high: old lineheight + the height of both new lines.
    // We can probably drop the old line height and start with height = 
@@ -2321,8 +2401,8 @@ wxLayoutList::FindObjectScreen(wxDC &dc, wxPoint const pos,
        cursorPos->y = line->GetLineNumber();
 
    // Now, find the object in the line:
-   ApplyStyle(line->GetStyleInfo(), dc);
+   wxLOiterator i = line->FindObjectScreen(dc, this,
-   wxLOiterator i = line->FindObjectScreen(dc, pos.x,
+                                           pos.x,
                                            cursorPos ? &cursorPos->x : NULL,
                                            found);
    return (i == NULLIT) ? NULL : *i;
@@ -2575,44 +2655,29 @@ wxLayoutList::DeleteSelection(void)
       return;
    }
 
+   // We now know that the two lines are different:
 
    wxLayoutLine
-      * firstLine = NULL,
+      * firstLine = GetLine(m_Selection.m_CursorA.y),
-      * lastLine = NULL;
+      * lastLine = GetLine(m_Selection.m_CursorB.y);
-
-   for(firstLine = m_FirstLine;
-       firstLine && firstLine->GetLineNumber() < m_Selection.m_CursorA.y;
-       firstLine=firstLine->GetNextLine())
-      ;
-   if(!firstLine || firstLine->GetLineNumber() != m_Selection.m_CursorA.y)
-      return;
-
-
-   for(lastLine = m_FirstLine;
-       lastLine && lastLine->GetLineNumber() < m_Selection.m_CursorB.y;
-       lastLine=lastLine->GetNextLine())
-      ;
-   if(!lastLine || lastLine->GetLineNumber() != m_Selection.m_CursorB.y)
-      return;
-
-
-   // We now know that the two lines are different:
 
    // First, delete what's left of this line:
    MoveCursorTo(m_Selection.m_CursorA);
    DeleteToEndOfLine();
 
-   wxLayoutLine *nextLine = firstLine->GetNextLine();
+   wxLayoutLine *prevLine = firstLine->GetPreviousLine(),
+                *nextLine = firstLine->GetNextLine();
    while(nextLine && nextLine != lastLine)
       nextLine = nextLine->DeleteLine(false, this);
 
    // Now nextLine = lastLine;
    Delete(1); // This joins firstLine and nextLine
-   Delete(m_Selection.m_CursorB.x); // This deletes the first x
+   Delete(m_Selection.m_CursorB.x); // This deletes the first x positions
-   // positions
 
-   /// Recalculate:
+   // Recalculate the line positions and numbers but notice that firstLine
-   firstLine->RecalculatePositions(1, this);
+   // might not exist any more - it could be deleted by Delete(1) above
+   wxLayoutLine *firstLine2 = prevLine ? prevLine->GetNextLine() : m_FirstLine;
+   firstLine2->RecalculatePositions(1, this);
 }
 
 /// Starts highlighting the selection

samples/richedit/wxllist.h

@@ -520,12 +520,14 @@ public:
    /** Finds the object which covers the screen position xpos in this
        line.
        @param dc the wxDC to use for calculations
+       @param llist the layout list to which this line belongs
        @param xpos the screen x coordinate
        @param offset where to store the difference between xpos and
        the object's head
        @return iterator to the object or NULLIT
    */
    wxLayoutObjectList::iterator FindObjectScreen(wxDC &dc,
+                                                 wxLayoutList *llist,
                                                  CoordType xpos,
                                                  CoordType *offset,
                                                  bool *found = NULL) const ;
@@ -541,11 +543,18 @@ public:
        functions to export the list.
        @return iterator to the first object
    */
-   wxLayoutObjectList::iterator GetFirstObject(void)
+   wxLayoutObjectList::iterator GetFirstObject(void) const
       {
          return m_ObjectList.begin();
       }
 
+   /** Get the last object in the list.
+    */
+   wxLayoutObjectList::iterator GetLastObject(void) const
+      {
+         return m_ObjectList.tail();
+      }
+
    /** Deletes this line, returns pointer to next line.
        @param update If true, update all following lines.
    */
@@ -600,8 +609,9 @@ public:
        for that position
        @return pointer to the object
    */
-   wxLayoutObject * FindObjectScreen(wxDC &dc, CoordType xpos, bool
+   wxLayoutObject * FindObjectScreen(wxDC &dc,
-                                     *found = NULL);
+                                     CoordType xpos,
+                                     bool *found = NULL);
    /** This sets the style info for the beginning of this line.
        @param si styleinfo structure
     */
@@ -1123,6 +1133,26 @@ public:
    void IncNumLines() { m_numLines++; }
    void DecNumLines() { m_numLines--; }
 
+   /// get the line by number
+   wxLayoutLine *GetLine(CoordType index) const
+   {
+       wxASSERT_MSG( (0 <= index) && (index < (CoordType)m_numLines),
+                     "invalid index" );
+
+       wxLayoutLine *line;
+       CoordType n = index;
+       for ( line = m_FirstLine; line && n-- > 0; line = line->GetNextLine() )
+           ;
+
+       if ( line )
+       {
+          // should be the right one
+          wxASSERT( line->GetLineNumber() == index );
+       }
+
+       return line;
+   }
+
 private:
    /// Clear the list.
    void InternalClear(void);

samples/richedit/wxlparser.cpp

@@ -41,10 +41,9 @@ void wxLayoutImportText(wxLayoutList *list, wxString const &str)
    if ( !str )
       return;
 
-   // we change the string temporarily inside this function
+   // we change the string only temporarily inside this function
-   wxString& s = (wxString &)str; // const_cast
+   // VZ: I still don't like it... the string data may be shared...
-
+   char * cptr = (char *)str.c_str(); // const_cast
-   char * cptr = s.GetWriteBuf(s.Len());
    const char * begin = cptr;
    char  backup;
 
@@ -70,8 +69,6 @@ void wxLayoutImportText(wxLayoutList *list, wxString const &str)
          break;
       cptr++;
    }
-
-   s.UngetWriteBuf();
 }
 
 static

samples/richedit/wxlwindow.cpp

@@ -140,7 +140,8 @@ wxLayoutWindow::wxLayoutWindow(wxWindow *parent)
                                  wxDefaultPosition, wxDefaultSize,
                                  wxHSCROLL | wxVSCROLL |
                                  wxBORDER |
-                                 wxWANTS_CHARS)
+                                 wxWANTS_CHARS),
+                m_llist(NULL)
 {
    SetStatusBar(NULL); // don't use statusbar
    m_Editable = false;
@@ -156,14 +157,18 @@ wxLayoutWindow::wxLayoutWindow(wxWindow *parent)
    m_BGbitmap = NULL;
    m_ScrollToCursor = false;
    SetWrapMargin(0);
+
+   // initially the list is empty, so why would we need the scrollbars?
+#if 0
    wxPoint max = m_llist->GetSize();
    SetScrollbars(X_SCROLL_PAGE, Y_SCROLL_PAGE,
                  max.x / X_SCROLL_PAGE + 1, max.y / Y_SCROLL_PAGE + 1);
    EnableScrolling(true, true);
    m_maxx = max.x + X_SCROLL_PAGE;
    m_maxy = max.y + Y_SCROLL_PAGE;
+#endif // 0
 
-   // no scrollbars initially (BTW, why then we do all the stuff above?)
+   // no scrollbars initially
    m_hasHScrollbar =
    m_hasVScrollbar = false;
 
@@ -579,6 +584,18 @@ wxLayoutWindow::OnChar(wxKeyEvent& event)
                   m_llist->WrapLine(m_WrapMargin);
                m_llist->LineBreak();
                break;
+
+            case WXK_TAB:
+               {
+                   // TODO should be configurable
+                   static const int tabSize = 8;
+
+                   CoordType x = m_llist->GetCursorPos().x;
+                   size_t numSpaces = tabSize - x % tabSize;
+                   m_llist->Insert(wxString(' ', numSpaces));
+               }
+               break;
+
             default:
                if((!(event.ControlDown() || event.AltDown() || event.MetaDown()))
                   && (keyCode < 256 && keyCode >= 32)
@@ -642,17 +659,8 @@ wxLayoutWindow::ScrollToCursor(void)
    // Get the size of the visible window:
    GetClientSize(&x1, &y1);
 
-   // notice that the client size may be (0, 0)...
+   // update the cursor screen position
-   wxASSERT(x1 >= 0 && y1 >= 0);
+   m_llist->Layout(dc);
-
-   // VZ: I think this is false - if you do it here, ResizeScrollbars() won't
-   //     call SetScrollbars() later
-#if 0
-   // As we have the values anyway, use them to avoid unnecessary scrollbar
-   // updates.
-   if(x1 > m_maxx) m_maxx = x1;
-   if(y1 > m_maxy) m_maxy = y1;
-#endif // 0
 
    // Make sure that the scrollbars are at a position so that the cursor is
    // visible if we are editing
@@ -859,8 +867,11 @@ wxLayoutWindow::InternalPaint(const wxRect *updateRect)
 
 void
 wxLayoutWindow::OnSize(wxSizeEvent &event)
+{
+   if ( m_llist )
    {
       ResizeScrollbars();
+   }
 
    event.Skip();
 }

samples/richedit/wxlwindow.h

@@ -23,7 +23,7 @@
 #endif
 
 
-#define wxUSE_PRIVATE_CLIPBOARD_FORMAT 1
+#define wxUSE_PRIVATE_CLIPBOARD_FORMAT 0
 
 enum
 {