in-place-editing patch for konqueror bookmarks

[Oelewapperke]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Wednesday 15 January 2003 01:40, David Faure wrote:
> On Sunday 29 December 2002 00:16, Oelewapperke wrote:
> > hi,
> >
> > my name is Christophe Devriese
> >
> > I have written a patch to kdelibs to give one the option of
> > right-clicking on a bookmark and selecting "delete" (implemented), or
> > "add" (to be added to the patch, this one is a proof of concept), etc.
> > This patch works without requiring any modifications to konqueror itself.
>
> Why does the patch seem to affect only KBookmarkBar?
> That's the "bookmark toolbar", but if all the code is there, then it means
> this feature isn't available in the normal bookmark menu....

indeed. I will post an additional patch to do that later. (btw I never ever 
use the menu except to get to "edit bookmarks").

> > I have a number of options to proceed
> > -> make kbookmark an abstract class, and force anyone wanting to use it
> > to create a subclass, then include the events on them as pure virtuals in
> > the abstract class
> > 	pro : 	-> this is, imho, the way it should be done
>
> Not IMHO. KBookmark is the _data_ only (a wrapper around QDomElement).
> All events and stuff are above it.
>
> > 			-> not binary compatible (but I don't know any app other than konq
> > that uses bookmarks)
>
> I think there are. Keeping BC is a no brainer anyway.

okay I understand that. Scratch this option

> > -> add a routine (or signal) to kbookmarkowner à la
> > MouseEvent(QMouseEvent qm, KBookmark bm)
> > 	pro :		-> "the quick hack", doesn't require changes to any other app
> > (that is kdelibs and konq need some changes)
> > 			-> one more commit will do it
> > 			-> binary compatible
> > 			-> I can (but it would be even uglier) avoid making any changes at all
> > to konqueror
>
> Changes in konq are fine.
>
> > 	con : 	-> slower
> > 			-> not extensible
> > 			-> complicated semantics without a very good reason (like having to
> > call QPopupMenu::exec and not QPopupMenu::popup)
>
> There are a lot of things in the patch that don't look very clean... e.g.
> + m_toolBar->insertWidget( 0, 25, new KIPEToolBarButton( m_toolBar, bm,
> m_pOwner ) ); 25? Why not 17 or 42? :)

the button changes that value anyway. I used 25, as it is the default value of 
some other class.

> Is the drawButton() stuff really necessary? What do those changes have to
> do with the way things are drawn? Can you explain the whole idea?

I couldn't use the normal KToolbarButton drawing routines, as they need the 
button to show a QPopupmenu that is constructed ahead of time, something I 
don't do (and why should I ? It slows the startup time, and the delay it 
creates when clicking a button can't be more than 0.1s (on my system))

> PS: please post diffs with "cvs diff -b" when they include lots of
> whitespace changes. Also, the diff doesn't include the contents of
> kipebookmarkmenu.h ...

sorry, this is my second patch ever, I'm still very much learning as I go.

I have completely reworked the patch, I will post it shortly (still need to 
implement "delete bookmark" again, because the way it's done now doesn't seem 
to save the altered bookmarks to disk)

Oelewapperke
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)

iD8DBQE+JWtoJFU6C5Uo8MgRAvAOAJ9Eu+J6yKsaXV1plQazRB1h60547ACfS7Rv
qTgylx/EViJ6gT1vTHikLKk=
=CrZD
-----END PGP SIGNATURE-----