Code review changes

include/wx/qt/private/treeitemdelegate.h

@@ -7,8 +7,8 @@
 // Licence:     wxWindows licence
 /////////////////////////////////////////////////////////////////////////////
 
-#ifndef _WX_TREEITEM_DELEGATE_H
-#define _WX_TREEITEM_DELEGATE_H
+#ifndef _WX_QT_PRIVATE_TREEITEM_DELEGATE_H
+#define _WX_QT_PRIVATE_TREEITEM_DELEGATE_H
 
 #include <QtWidgets/QStyledItemDelegate>
 
@@ -28,7 +28,10 @@ public:
 
     QWidget* createEditor(QWidget *parent, const QStyleOptionViewItem &WXUNUSED(option), const QModelIndex &index) const wxOVERRIDE
     {
-        m_current_model_index = index;
+        if ( m_textCtrl != NULL )
+            destroyEditor(m_textCtrl->GetHandle(), m_currentModelIndex);
+
+        m_currentModelIndex = index;
         m_textCtrl = new wxQtListTextCtrl(m_parent, parent);
         m_textCtrl->SetFocus();
         return m_textCtrl->GetHandle();
@@ -36,9 +39,9 @@ public:
 
     void destroyEditor(QWidget *WXUNUSED(editor), const QModelIndex &WXUNUSED(index)) const wxOVERRIDE
     {
-        if (!wxTheApp->IsScheduledForDestruction(m_textCtrl))
+        if ( m_textCtrl != NULL )
         {
-            m_current_model_index = QModelIndex();
+            m_currentModelIndex = QModelIndex(); // invalidate the index
             wxTheApp->ScheduleForDestruction(m_textCtrl);
             m_textCtrl = NULL;
         }
@@ -56,7 +59,7 @@ public:
 
     QModelIndex GetCurrentModelIndex() const
     {
-        return m_current_model_index;
+        return m_currentModelIndex;
     }
 
     void AcceptModelData(QWidget *editor, QAbstractItemModel *model, const QModelIndex &index) const
@@ -67,7 +70,7 @@ public:
 private:
     wxWindow* m_parent;
     mutable wxTextCtrl* m_textCtrl;
-    mutable QModelIndex m_current_model_index;
+    mutable QModelIndex m_currentModelIndex;
 };
 
-#endif // _WX_TREEITEM_DELEGATE_H
+#endif // _WX_QT_PRIVATE_TREEITEM_DELEGATE_H

include/wx/qt/private/treeitemfactory.h

@@ -7,8 +7,8 @@
 // Licence:     wxWindows licence
 /////////////////////////////////////////////////////////////////////////////
 
-#ifndef _WX_TREEITEM_FACTORY_H_
-#define _WX_TREEITEM_FACTORY_H_
+#ifndef _WX_QT_PRIVATE_TREEITEM_FACTORY_H_
+#define _WX_QT_PRIVATE_TREEITEM_FACTORY_H_
 
 #include <QtWidgets/QItemEditorFactory>
 #include <QtWidgets/QTreeWidget>
@@ -91,11 +91,11 @@ public:
         qItemDelegate->setItemEditorFactory(this);
     }
 
-    virtual QWidget* createEditor(
-        int WXUNUSED(userType),
-        QWidget* parent
-    ) const wxOVERRIDE
+    QWidget* createEditor(int WXUNUSED(userType), QWidget* parent) const wxOVERRIDE
     {
+        if (m_textCtrl != NULL)
+            ClearEditor();
+
         m_textCtrl = new wxQtListTextCtrl(m_parent, parent);
         m_textCtrl->SetFocus();
         return m_textCtrl->GetHandle();
@@ -106,7 +106,7 @@ public:
         return m_textCtrl;
     }
 
-    void ClearEditor()
+    void ClearEditor() const
     {
         delete m_textCtrl;
         m_textCtrl = NULL;
@@ -119,4 +119,4 @@ private:
     wxDECLARE_NO_COPY_CLASS(wxQtTreeItemEditorFactory);
 };
 
-#endif //_WX_TREEITEM_FACTORY_H_
+#endif //_WX_QT_PRIVATE_TREEITEM_FACTORY_H_

include/wx/qt/treectrl.h

@@ -75,7 +75,6 @@ public:
     virtual wxTreeItemId GetFocusedItem() const wxOVERRIDE;
 
     virtual wxTreeItemId GetItemParent(const wxTreeItemId& item) const wxOVERRIDE;
-     
     virtual wxTreeItemId GetFirstChild(const wxTreeItemId& item, wxTreeItemIdValue& cookie) const wxOVERRIDE;
     virtual wxTreeItemId GetNextChild(const wxTreeItemId& item, wxTreeItemIdValue& cookie) const wxOVERRIDE;
     virtual wxTreeItemId GetLastChild(const wxTreeItemId& item) const wxOVERRIDE;

src/qt/treectrl.cpp

@@ -16,6 +16,7 @@
 #include "wx/imaglist.h"
 #include "wx/settings.h"
 #include "wx/sharedptr.h"
+#include "wx/withimages.h"
 
 #include "wx/qt/private/winevent.h"
 #include "wx/qt/private/treeitemdelegate.h"
@@ -32,12 +33,13 @@ struct TreeItemDataQt
     {
     }
 
-    TreeItemDataQt(wxTreeItemData* data) : data(data)
+    explicit TreeItemDataQt(wxTreeItemData* data) : data(data)
     {
-        if ( !registered )
+        static bool s_registered = false;
+        if ( !s_registered )
         {
             qRegisterMetaTypeStreamOperators<TreeItemDataQt>("TreeItemDataQt");
-            registered = true;
+            s_registered = true;
         }
     }
 
@@ -48,11 +50,8 @@ struct TreeItemDataQt
 
 private:
     wxSharedPtr<wxTreeItemData> data;
-    static bool registered;
 };
 
-bool TreeItemDataQt::registered = false;
-
 QDataStream &operator<<(QDataStream &out, const TreeItemDataQt &WXUNUSED(obj))
 {
     return out;
@@ -84,6 +83,7 @@ size_t CountChildren(QTreeWidgetItem *item)
 
     return totalCount;
 }
+
 class ImageState
 {
 public:
@@ -91,7 +91,7 @@ public:
     {
         for ( int i = wxTreeItemIcon_Normal; i < wxTreeItemIcon_Max; ++i )
         {
-            m_imageStates[i] = -1;
+            m_imageStates[i] = wxWithImages::NO_IMAGE;
         }
     }
 
@@ -101,7 +101,7 @@ public:
         return m_imageStates[index];
     }
 
-    const int &operator[](size_t index) const
+    int operator[](size_t index) const
     {
         wxASSERT(index < wxTreeItemIcon_Max);
         return m_imageStates[index];
@@ -587,7 +587,7 @@ void wxTreeCtrl::SetStateImageList(wxImageList *imageList)
 wxString wxTreeCtrl::GetItemText(const wxTreeItemId& item) const
 {
     if ( !item.IsOk() )
-        return "";
+        return wxString();
 
     const QTreeWidgetItem* qTreeItem = wxQtConvertTreeItem(item);
     return wxQtConvertString(qTreeItem->text(0));
@@ -816,10 +816,11 @@ size_t wxTreeCtrl::GetSelections(wxArrayTreeItemIds& selections) const
     QList<QTreeWidgetItem*> qtSelections = m_qtTreeWidget->selectedItems();
 
     const size_t numberOfSelections = qtSelections.size();
+    selections.reserve(numberOfSelections);
     for ( size_t i = 0; i < numberOfSelections; ++i )
     {
         QTreeWidgetItem *item = qtSelections[i];
-        selections.Add(wxQtConvertTreeItem(item));
+        selections.push_back(wxQtConvertTreeItem(item));
     }
 
     return numberOfSelections;