Qt: playlist model cleanup

Remove unneeded parameters, Privatize methods and members, get closer to
Qt namings and QAbstractItemModel habits, comments

modules/gui/qt4/components/playlist/playlist_model.cpp

@@ -311,7 +311,8 @@ void PLModel::activateItem( const QModelIndex &index )
     PL_UNLOCK;
 }
 
-/* Must be entered with lock */
+/* Convenient overloaded private version of activateItem
+ * Must be entered with PL lock */
 void PLModel::activateItem( playlist_item_t *p_item )
 {
     if( !p_item ) return;
@@ -892,23 +893,22 @@ bool PLModel::popup( const QModelIndex & index, const QPoint &point, const QMode
         return false;
     }
 
-    input_item_t *p_input = p_item->p_input;
-    vlc_gc_incref( p_input );
-
-    i_popup_item = index.isValid() ? p_item->i_id : -1;
+    i_popup_item   = index.isValid() ? p_item->i_id : -1;
     i_popup_parent = index.isValid() ?
         ( p_item->p_parent ? p_item->p_parent->i_id : -1 ) :
         ( rootItem->id() );
-    i_popup_column = index.column();
 
     bool tree = ( rootItem && rootItem->id() != p_playlist->p_playing->i_id ) ||
                 var_InheritBool( p_intf, "playlist-tree" );
 
+    input_item_t *p_input = p_item->p_input;
+    vlc_gc_incref( p_input );
     PL_UNLOCK;
 
-    current_selection = list;
-
+    /* */
     QMenu menu;
+
+    /* Play/Stream/Info static actions */
     if( i_popup_item > -1 )
     {
         menu.addAction( QIcon( ":/menu/play" ), qtr(I_POP_PLAY), this, SLOT( popupPlay() ) );
@@ -925,6 +925,7 @@ bool PLModel::popup( const QModelIndex & index, const QPoint &point, const QMode
     }
     vlc_gc_decref( p_input );
 
+    /* In PL or ML, allow to add a file/folder */
     if( canEdit() )
     {
         QIcon addIcon( ":/buttons/playlist/playlist_add" );
@@ -944,6 +945,8 @@ bool PLModel::popup( const QModelIndex & index, const QPoint &point, const QMode
             menu.addAction( addIcon, qtr(I_OP_ADVOP), THEDP, SLOT( MLAppendDialog() ) );
         }
     }
+
+    /* Item removal */
     if( i_popup_item > -1 )
     {
         if( rootItem->id() != THEPL->p_playing->i_id )
@@ -951,26 +954,31 @@ bool PLModel::popup( const QModelIndex & index, const QPoint &point, const QMode
         menu.addAction( QIcon( ":/buttons/playlist/playlist_remove" ),
                         qtr(I_POP_DEL), this, SLOT( popupDel() ) );
         menu.addSeparator();
-        if( !sortingMenu )
+    }
+
+    /* Playlist sorting */
+    if( !sortingMenu )
+    {
+        sortingMenu = new QMenu( qtr( "Sort by" ) );
+        sortingMapper = new QSignalMapper( this );
+        for( int i = 1, j = 1; i < COLUMN_ALBUM; i <<= 1, j++ )
         {
-            sortingMenu = new QMenu( qtr( "Sort by" ) );
-            sortingMapper = new QSignalMapper( this );
-            for( int i = 1, j = 1; i < COLUMN_END; i <<= 1, j++ )
-            {
-                if( i == COLUMN_NUMBER ) continue;
-                QMenu *m = sortingMenu->addMenu( qfu( psz_column_title( i ) ) );
-                QAction *asc = m->addAction( qtr("Ascending") );
-                QAction *desc = m->addAction( qtr("Descending") );
-                sortingMapper->setMapping( asc, j );
-                sortingMapper->setMapping( desc, -j );
-                CONNECT( asc, triggered(), sortingMapper, map() );
-                CONNECT( desc, triggered(), sortingMapper, map() );
-            }
-            CONNECT( sortingMapper, mapped( int ), this, popupSort( int ) );
+            if( i == COLUMN_NUMBER ) continue;
+            QAction *asc  = sortingMenu->addAction( qfu( psz_column_title( i ) ) + " " + qtr("Ascending") );
+            QAction *desc = sortingMenu->addAction( qfu( psz_column_title( i ) ) + " " + qtr("Descending") );
+            sortingMapper->setMapping( asc, j );
+            sortingMapper->setMapping( desc, -j );
+            CONNECT( asc, triggered(), sortingMapper, map() );
+            CONNECT( desc, triggered(), sortingMapper, map() );
         }
-        menu.addMenu( sortingMenu );
+        CONNECT( sortingMapper, mapped( int ), this, popupSort( int ) );
     }
+    menu.addMenu( sortingMenu );
+
+    /* Store the current selected item for popup*() methods */
+    current_selection = list;
 
+    /* Display and forward the result */
     if( !menu.isEmpty() )
     {
         menu.exec( point ); return true;
@@ -1000,7 +1008,7 @@ void PLModel::popupAddToPlaylist()
 
     foreach( QModelIndex currentIndex, current_selection )
     {
-        playlist_item_t *p_item = playlist_ItemGetById( THEPL, getId( currentIndex ) );
+        playlist_item_t *p_item = playlist_ItemGetById( THEPL, itemId( currentIndex ) );
         if( !p_item ) continue;
 
         playlist_NodeAddCopy( THEPL, p_item,

modules/gui/qt4/components/playlist/playlist_model.hpp

@@ -56,6 +56,7 @@ public:
              playlist_item_t *, QObject *parent = 0 );
     virtual ~PLModel();
 
+    /* Qt4 main PLModel */
     static PLModel* getPLModel( intf_thread_t *p_intf )
     {
         if(!p_intf->p_sys->pl_model )
@@ -69,7 +70,7 @@ public:
         return p_intf->p_sys->pl_model;
     }
 
-    /*** QModel subclassing ***/
+    /*** QAbstractItemModel subclassing ***/
 
     /* Data structure */
     virtual QVariant data( const QModelIndex &index, const int role ) const;
@@ -88,36 +89,31 @@ public:
                       int row, int column, const QModelIndex &target );
     virtual QStringList mimeTypes() const;
 
+    /* Sort */
+    virtual void sort( const int column, Qt::SortOrder order = Qt::AscendingOrder );
+
     /**** Custom ****/
 
     /* Lookups */
-    QStringList selectedURIs();
-    QModelIndex index( PLItem *, const int c ) const;
     QModelIndex index( const int i_id, const int c );
     virtual QModelIndex currentIndex() const;
-    bool isParent( const QModelIndex &index, const QModelIndex &current) const;
-    bool isCurrent( const QModelIndex &index ) const;
     int itemId( const QModelIndex &index ) const;
 
-    /* Actions */
-    virtual bool popup( const QModelIndex & index, const QPoint &point, const QModelIndexList &list );
-    virtual void doDelete( QModelIndexList selected );
+    /* */
     void search( const QString& search_text, const QModelIndex & root, bool b_recursive );
-    void sort( const int column, Qt::SortOrder order );
-    void sort( const int i_root_id, const int column, Qt::SortOrder order );
     void rebuild( playlist_item_t * p = NULL );
 
-    inline PLItem *getItem( QModelIndex index ) const
+    /* Popup Actions */
+    virtual bool popup( const QModelIndex & index, const QPoint &point, const QModelIndexList &list );
+    virtual void doDelete( QModelIndexList selected );
+
+    PLItem *getItem( const QModelIndex & index ) const
     {
         if( index.isValid() )
             return static_cast<PLItem*>( index.internalPointer() );
         else return rootItem;
     }
-    virtual int getId( QModelIndex index ) const
-    {
-        return getItem( index )->id();
-    }
-    inline int getZoom() const
+    int getZoom() const
     {
         return i_zoom;
     }
@@ -128,8 +124,7 @@ signals:
 
 public slots:
     virtual void activateItem( const QModelIndex &index );
-    void activateItem( playlist_item_t *p_item );
-    inline void changeZoom( const int zoom )
+    void changeZoom( const int zoom )
     {
         i_zoom = zoom;
         emit layoutChanged();
@@ -143,6 +138,13 @@ private:
 
     static QIcon icons[ITEM_TYPE_NUMBER];
 
+    /* Custom model private methods */
+    /* Lookups */
+    QStringList selectedURIs();
+    QModelIndex index( PLItem *, const int c ) const;
+    bool isCurrent( const QModelIndex &index ) const;
+    bool isParent( const QModelIndex &index, const QModelIndex &current) const;
+
     /* Shallow actions (do not affect core playlist) */
     void updateTreeItem( PLItem * );
     void removeItem ( PLItem * );
@@ -158,8 +160,11 @@ private:
     void dropAppendCopy( const PlMimeData * data, PLItem *target, int pos );
     void dropMove( const PlMimeData * data, PLItem *target, int new_pos );
 
+    /* */
+    void sort( const int i_root_id, const int column, Qt::SortOrder order );
+
     /* Popup */
-    int i_popup_item, i_popup_parent, i_popup_column;
+    int i_popup_item, i_popup_parent;
     QModelIndexList current_selection;
     QMenu *sortingMenu;
     QSignalMapper *sortingMapper;
@@ -192,6 +197,7 @@ private slots:
     void processInputItemUpdate( input_thread_t* p_input );
     void processItemRemoval( int i_id );
     void processItemAppend( int item, int parent );
+    void activateItem( playlist_item_t *p_item );
 };
 
 class PlMimeData : public QMimeData

modules/gui/qt4/components/playlist/vlc_model.hpp

@@ -50,7 +50,7 @@ public:
     };
 
     VLCModel( intf_thread_t *_p_intf, QObject *parent = 0 );
-    virtual int getId( QModelIndex index ) const = 0;
+    virtual int itemId( const QModelIndex & ) const = 0;
     virtual QModelIndex currentIndex() const = 0;
     virtual bool popup( const QModelIndex & index,
             const QPoint &point, const QModelIndexList &list ) = 0;
@@ -61,8 +61,7 @@ public:
 
     static int columnToMeta( int _column )
     {
-        int meta = 1;
-        int column = 0;
+        int meta = 1, column = 0;
 
         while( column != _column && meta != COLUMN_END )
         {
@@ -75,8 +74,7 @@ public:
 
     static int columnFromMeta( int meta_col )
     {
-        int meta = 1;
-        int column = 0;
+        int meta = 1, column = 0;
 
         while( meta != meta_col && meta != COLUMN_END )
         {