[PATCH] KFilePlacesModel : separate storage for custom items and standard items

[Kevin Ottens]

Le Sunday 09 March 2008, nf2 a écrit :
> Here is an experimental patch for KFilePlacesModel, which separates the
> storage for the custom items and the standard ("root") items.

OK, let's start with the cosmetic comments:

 - I wouldn't call the standard items "root" in the code, that's IMO 
confusing, I'd expect them to be at the root of the filesystem and that's not 
the case. I'd call them "system", "vendor", or something like this.

 - In KFilePlacesModel if you keep two KBookmarkManager you should probably 
give them better names (bookmarkManager vs bookmarkManagerRoot lacks of 
distinction).

 - reloadAndSignals(bool) should be two methods, or take the bookmark manager 
as argument instead.

 - I noticed the KStandardDirs().localxdgdatadir(), I'm wondering but maybe 
we're lacking a "xdgdata" resource name for KStandardDirs, it might be worth 
to add maybe?

 - Also be careful at the coding style and the indentation, I spotted and 
indentation mistake line 166 or your patch, and the if() in 
reloadAndSignals() doesn't comply to kdelibs coding standard.

> The custom items go to ~/.local/share/filemgr-shortcuts.xbel, that they
> can be shared with other desktops, and the root items are stored in
> ~/.kde4/share/apps/kfileplaces/rootbookmarks.xml.

I'd put ~/.local/share/filemgr-shortcuts.xbel somewhere else to avoid 
cluttering ~/.local/share, a subfolder in apps perhaps?

> This patch depends on my previous KBookmarks patch (to enable KDirWatch
> monitoring for external bookmarks).

OK, I applied your patches, and it totally blows the unit tests for the class, 
so please rework your patch and ensure it at least passes the tests.

> I still have a little problem that <IsHidden> is not always written to
> the files correctly, but i can't figure out where the bug is.

Well, there's also a few more issues (actually catched by the tests). For 
instance I can't reorder my places anymore, the ones living in .local are 
necessarily at the end of the list.

Regards.
-- 
Kévin 'ervin' Ottens, http://ervin.ipsquad.net
"Ni le maître sans disciple, Ni le disciple sans maître,
Ne font reculer l'ignorance."

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 194 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20080311/7abf3bfa/attachment.sig>