------=_Part_1352_1261150.1146111547735 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline For the bookmark menu, I am using a slightly better constructor: RMB(KBookmarkMenu *target) I need to use the KBookmarkMenu * because (QMenu) contextMenu->addAction(...) uses recv all over the place in kbookmarkmenu.cc, so it's not as cool as the bookmarkbar solution. Also, I'm making assumptions about KBookmarkMenu::d working the same as KBookmarkBar::d. However, the former is in the header, but not implemented !?!?! Example in kbookmarkmenu.cc: void KBookmarkMenu::slotRMBActionEditAt( int val ) { delete d->m_rmb; d->m_rmb =3D new RMB(this); rmbSelf(this)->slotRMBActionProperties( val ); } Please advise on what to do here, 1) don't use d->m_rmb 2) use d->m_rmb instead of rmbSelf(this) As far as replacing all dPtrs: Without understanding much about the code, I intend to replace them with QHashes. I think I'm starting to understand this code, and at that, I would recommend that KBookmarkMenu get a RMB *m_RMB and KBookmarkManager get a KBookmarkManagerPrivate * m_kBookmarkManagerPrivate. What are your thoughts here? =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3Dthe chase=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D So here's my progress with cleaning up bookmarks as per TODO #50: x Replace begin_rmb_action(KBookmarkBar *) x Replace begin_rmb_action(KBookmarkMenu *) _ Replace all dPtrs x Rename RMB::s_highlightedAddress Cheers, - Will Entriken ------=_Part_1352_1261150.1146111547735 Content-Type: application/octet-stream; name=bookmarkscleanup.patch Content-Transfer-Encoding: 7bit X-Attachment-Id: f_emilh3d6 Content-Disposition: attachment; filename="bookmarkscleanup.patch" Index: kbookmarkbar.cc =================================================================== --- kbookmarkbar.cc (revision 533892) +++ kbookmarkbar.cc (working copy) @@ -403,34 +403,51 @@ // TODO *** generic rmb improvements *** // don't *ever* show the rmb on press, always relase, possible??? - -void RMB::begin_rmb_action(KBookmarkBar *self) +RMB::RMB(QString parentAddress, QString highlightedAddress, + KBookmarkManager *pManager, KBookmarkOwner *pOwner) { - if ( !self->d->m_rmb ) - self->d->m_rmb = new RMB; - RMB *s = self->d->m_rmb; - s->recv = self; - s->m_parentAddress = self->parentAddress(); - s->s_highlightedAddress = self->d->m_highlightedAddress; // rename in RMB - s->m_pManager = self->m_pManager; - s->m_pOwner = self->m_pOwner; - s->m_parentMenu = 0; + m_parentAddress = parentAddress; + m_highlightedAddress = highlightedAddress; + m_pManager = pManager; + m_pOwner = pOwner; + m_parentMenu = 0; } void KBookmarkBar::slotRMBActionEditAt( int val ) -{ RMB::begin_rmb_action(this); d->m_rmb->slotRMBActionEditAt( val ); } +{ + delete d->m_rmb; + d->m_rmb = new RMB(parentAddress(), d->m_highlightedAddress, m_pManager, m_pOwner); + d->m_rmb->slotRMBActionEditAt( val ); +} void KBookmarkBar::slotRMBActionProperties( int val ) -{ RMB::begin_rmb_action(this); d->m_rmb->slotRMBActionProperties( val ); } +{ + delete d->m_rmb; + d->m_rmb = new RMB(patentAddress(), d->m_highlightedAddress, m_pManager, m_pOwner); + d->m_rmb->slotRMBActionProperties( val ); +} void KBookmarkBar::slotRMBActionInsert( int val ) -{ RMB::begin_rmb_action(this); d->m_rmb->slotRMBActionInsert( val ); } +{ + delete d->m_rmb; + d->m_rmb = new RMB(patentAddress(), d->m_highlightedAddress, m_pManager, m_pOwner); + d->m_rmb->slotRMBActionInsert( val ); +} + void KBookmarkBar::slotRMBActionRemove( int val ) -{ RMB::begin_rmb_action(this); d->m_rmb->slotRMBActionRemove( val ); } +{ + delete d->m_rmb; + d->m_rmb = new RMB(patentAddress(), d->m_highlightedAddress, m_pManager, m_pOwner); + d->m_rmb->slotRMBActionRemove( val ); +} void KBookmarkBar::slotRMBActionCopyLocation( int val ) -{ RMB::begin_rmb_action(this); d->m_rmb->slotRMBActionCopyLocation( val ); } +{ + delete d->m_rmb; + d->m_rmb = new RMB(patentAddress(), d->m_highlightedAddress, m_pManager, m_pOwner); + d->m_rmb->slotRMBActionCopyLocation( val ); +} bool KBookmarkBar::eventFilter( QObject *o, QEvent *e ) { @@ -451,7 +468,8 @@ { d->m_highlightedAddress = _a->property("address").toString(); KBookmark bookmark = m_pManager->findByAddress( d->m_highlightedAddress ); - RMB::begin_rmb_action(this); + delete d->m_rmb; + d->m_rmb = new RMB(patentAddress(), d->m_highlightedAddress, m_pManager, m_pOwner); KMenu *pm = new KMenu; d->m_rmb->fillContextMenu( pm, d->m_highlightedAddress, 0 ); emit aboutToShowContextMenu( d->m_rmb->atAddress( d->m_highlightedAddress ), pm ); Index: kbookmarkmenu.cc =================================================================== --- kbookmarkmenu.cc (revision 533892) +++ kbookmarkmenu.cc (working copy) @@ -157,7 +157,7 @@ } } -QString KBookmarkMenu::s_highlightedAddress; +QString KBookmarkMenu::m_highlightedAddress; QString KBookmarkMenu::s_highlightedImportType; QString KBookmarkMenu::s_highlightedImportLocation; @@ -166,8 +166,8 @@ if (qobject_cast(action) || qobject_cast(action)) { - s_highlightedAddress = action->property("address").toString(); - //kDebug() << "KBookmarkMenu::slotActionHighlighted" << s_highlightedAddress << endl; + m_highlightedAddress = action->property("address").toString(); + //kDebug() << "KBookmarkMenu::slotActionHighlighted" << m_highlightedAddress << endl; } else if (qobject_cast(action)) { @@ -176,7 +176,7 @@ } else { - s_highlightedAddress.clear(); + m_highlightedAddress.clear(); s_highlightedImportType.clear(); s_highlightedImportLocation.clear(); } @@ -189,21 +189,21 @@ // TODO use d pointer instead - or add a pointer for the RMB instance. class KBookmarkMenuRMBAssoc : public dPtrTemplate { }; -template<> QHash* dPtrTemplate::d_ptr = 0; +template<> QHash* dPtrTemplate::d_ptr = 0; //memory leak!? static RMB* rmbSelf(KBookmarkMenu *m) { return KBookmarkMenuRMBAssoc::d(m); } // TODO check via dcop before making any changes to the bookmarks file??? -void RMB::begin_rmb_action(KBookmarkMenu *self) +RMB::RMB(KBookmarkMenu *target) { - RMB *s = rmbSelf(self); - s->recv = self; - s->m_parentAddress = self->m_parentAddress; - s->s_highlightedAddress = KBookmarkMenu::s_highlightedAddress; - s->m_pManager = self->m_pManager; - s->m_pOwner = self->m_pOwner; - s->m_parentMenu = self->m_parentMenu; + RMB *s = rmbSelf(target); + s->recv = target; + s->m_parentAddress = target->m_parentAddress; + s->m_highlightedAddress = KBookmarkMenu::m_highlightedAddress; + s->m_pManager = target->m_pManager; + s->m_pOwner = target->m_pOwner; + s->m_parentMenu = target->m_parentMenu; } bool RMB::invalid( int val ) @@ -211,9 +211,9 @@ bool valid = true; if (val == 1) - s_highlightedAddress = m_parentAddress; + m_highlightedAddress = m_parentAddress; - if (s_highlightedAddress.isNull()) + if (m_highlightedAddress.isNull()) valid = false; return !valid; @@ -228,14 +228,14 @@ void KBookmarkMenu::slotAboutToShowContextMenu( KMenu*, QAction*, QMenu* contextMenu ) { - //kDebug(7043) << "KBookmarkMenu::slotAboutToShowContextMenu" << s_highlightedAddress << endl; - if (s_highlightedAddress.isNull()) + //kDebug(7043) << "KBookmarkMenu::slotAboutToShowContextMenu" << m_highlightedAddress << endl; + if (m_highlightedAddress.isNull()) { KMenu::contextMenuFocus()->hideContextMenu(); return; } contextMenu->clear(); - fillContextMenu( contextMenu, s_highlightedAddress, 0 ); + fillContextMenu( contextMenu, m_highlightedAddress, 0 ); } void RMB::fillContextMenu( QMenu* contextMenu, const QString & address, int val ) @@ -292,20 +292,20 @@ void RMB::slotRMBActionEditAt( int val ) { - kDebug(7043) << "KBookmarkMenu::slotRMBActionEditAt" << s_highlightedAddress << endl; + kDebug(7043) << "KBookmarkMenu::slotRMBActionEditAt" << m_highlightedAddress << endl; if (invalid(val)) { hidePopup(); return; } - KBookmark bookmark = atAddress(s_highlightedAddress); + KBookmark bookmark = atAddress(m_highlightedAddress); - m_pManager->slotEditBookmarksAtAddress( s_highlightedAddress ); + m_pManager->slotEditBookmarksAtAddress( m_highlightedAddress ); } void RMB::slotRMBActionProperties( int val ) { - kDebug(7043) << "KBookmarkMenu::slotRMBActionProperties" << s_highlightedAddress << endl; + kDebug(7043) << "KBookmarkMenu::slotRMBActionProperties" << m_highlightedAddress << endl; if (invalid(val)) { hidePopup(); return; } - KBookmark bookmark = atAddress(s_highlightedAddress); + KBookmark bookmark = atAddress(m_highlightedAddress); QString folder = bookmark.isGroup() ? QString() : bookmark.url().pathOrURL(); KBookmarkEditDialog dlg( bookmark.fullText(), folder, @@ -332,7 +332,7 @@ void RMB::slotRMBActionInsert( int val ) { - kDebug(7043) << "KBookmarkMenu::slotRMBActionInsert" << s_highlightedAddress << endl; + kDebug(7043) << "KBookmarkMenu::slotRMBActionInsert" << m_highlightedAddress << endl; if (invalid(val)) { hidePopup(); return; } QString url = m_pOwner->currentURL(); @@ -345,7 +345,7 @@ if (title.isEmpty()) title = url; - KBookmark bookmark = atAddress( s_highlightedAddress ); + KBookmark bookmark = atAddress( m_highlightedAddress ); // TODO use unique title @@ -368,10 +368,10 @@ void RMB::slotRMBActionRemove( int val ) { - //kDebug(7043) << "KBookmarkMenu::slotRMBActionRemove" << s_highlightedAddress << endl; + //kDebug(7043) << "KBookmarkMenu::slotRMBActionRemove" << m_highlightedAddress << endl; if (invalid(val)) { hidePopup(); return; } - KBookmark bookmark = atAddress( s_highlightedAddress ); + KBookmark bookmark = atAddress( m_highlightedAddress ); bool folder = bookmark.isGroup(); if (KMessageBox::warningContinueCancel( @@ -394,10 +394,10 @@ void RMB::slotRMBActionCopyLocation( int val ) { - //kDebug(7043) << "KBookmarkMenu::slotRMBActionCopyLocation" << s_highlightedAddress << endl; + //kDebug(7043) << "KBookmarkMenu::slotRMBActionCopyLocation" << m_highlightedAddress << endl; if (invalid(val)) { hidePopup(); return; } - const KBookmark bookmark = atAddress( s_highlightedAddress ); + const KBookmark bookmark = atAddress( m_highlightedAddress ); if ( !bookmark.isGroup() ) { @@ -420,26 +420,48 @@ void KBookmarkMenu::fillContextMenu( QMenu* contextMenu, const QString & address, int val ) { - RMB::begin_rmb_action(this); + delete d->m_rmb; + d->m_rmb = new RMB(this); + rmbSelf(this)->fillContextMenu(contextMenu, address, val); emit aboutToShowContextMenu( rmbSelf(this)->atAddress(address), contextMenu); rmbSelf(this)->fillContextMenu2(contextMenu, address, val); } void KBookmarkMenu::slotRMBActionEditAt( int val ) -{ RMB::begin_rmb_action(this); rmbSelf(this)->slotRMBActionEditAt( val ); } +{ + delete d->m_rmb; + d->m_rmb = new RMB(this); + rmbSelf(this)->slotRMBActionProperties( val ); +} void KBookmarkMenu::slotRMBActionProperties( int val ) -{ RMB::begin_rmb_action(this); rmbSelf(this)->slotRMBActionProperties( val ); } +{ + delete d->m_rmb; + d->m_rmb = new RMB(this); + rmbSelf(this)->slotRMBActionProperties( val ); +} void KBookmarkMenu::slotRMBActionInsert( int val ) -{ RMB::begin_rmb_action(this); rmbSelf(this)->slotRMBActionInsert( val ); } +{ + delete d->m_rmb; + d->m_rmb = new RMB(this); + rmbSelf(this)->slotRMBActionProperties( val ); +} void KBookmarkMenu::slotRMBActionRemove( int val ) -{ RMB::begin_rmb_action(this); rmbSelf(this)->slotRMBActionRemove( val ); } +{ + delete d->m_rmb; + d->m_rmb = new RMB(this); + rmbSelf(this)->slotRMBActionProperties( val ); +} void KBookmarkMenu::slotRMBActionCopyLocation( int val ) -{ RMB::begin_rmb_action(this); rmbSelf(this)->slotRMBActionCopyLocation( val ); } +{ + delete d->m_rmb; + d->m_rmb = new RMB(this); + rmbSelf(this)->slotRMBActionProperties( val ); +} void KBookmarkMenu::slotBookmarksChanged( const QString & groupAddress ) { Index: dptrtemplate.h =================================================================== --- dptrtemplate.h (revision 533892) +++ dptrtemplate.h (working copy) @@ -24,9 +24,11 @@ #include +/* A lazily-created hash from Instance to PrivateData */ template class dPtrTemplate { public: + /* get hash[instance] */ static PrivateData* d( const Instance* instance ) { if ( !d_ptr ) { @@ -40,10 +42,11 @@ } return ret; } + /* remove hash[instance] */ static void delete_d( const Instance* instance ) { if ( d_ptr ) - delete d_ptr->take( instance ); + delete d_ptr->remove( instance ); } private: static void cleanup_d_ptr() Index: kbookmarkmenu.h =================================================================== --- kbookmarkmenu.h (revision 533892) +++ kbookmarkmenu.h (working copy) @@ -218,7 +218,7 @@ class KBookmarkMenuPrivate* d; // TODO make non static! - static QString s_highlightedAddress; + static QString m_highlightedAddress; static QString s_highlightedImportLocation; static QString s_highlightedImportType; }; Index: kbookmarkmenu_p.h =================================================================== --- kbookmarkmenu_p.h (revision 533892) +++ kbookmarkmenu_p.h (working copy) @@ -190,12 +190,14 @@ static KBookmarkSettings *self(); }; +/* Right mouse button */ class RMB { public: - // ##### DF: How about two normal constructors instead?? - static void begin_rmb_action(KBookmarkMenu *); - static void begin_rmb_action(KBookmarkBar *); + RMB(QString parentAddress, QString highlightedAddress, + KBookmarkManager *pManager, KBookmarkOwner *pOwner); + RMB(KBookmarkMenu *target); + bool invalid( int val ); KBookmark atAddress(const QString & address); void fillContextMenu( QMenu* contextMenu, const QString & address, int val ); @@ -209,7 +211,7 @@ public: QObject *recv; KBookmarkManager *m_pManager; - QString s_highlightedAddress; + QString m_highlightedAddress; QString m_parentAddress; KBookmarkOwner *m_pOwner; QWidget *m_parentMenu; ------=_Part_1352_1261150.1146111547735 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline >> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe << ------=_Part_1352_1261150.1146111547735--