Re: [KMess-devel] Suggestions for improvement on the Gitorious merge request

[Sjors G. <sj...@km...>]

Op 13 dec 2010, om 01:56 heeft Daniel E. Moctezuma het volgende geschreven:

> Hi Sjors (and all KMess devs),

Hi Daniel,

Sorry for responding to you this late :( I have free time from school now, so hopefully we can get that patch merged finally ;-)

> Long time, no see. I've been more in real life (school) than in coding, but finally got time to clean my previous patch.
> Here I'm sending it for you to check it and get some feedback if possible.
> I'm not including 1 of 3 plugins (predefined sentences) as is the plugin that makes use of the icons, and as you mentioned:
> <normaloff>../../../../../../usr/share/icons/oxygen/16x16/categories/preferences-system.png</normaloff>../../../../../../usr/share/icons/oxygen/16x16/categories/preferences-system.png</iconset>
> 
> the path should be "preferences-system" only. I'm working on that and decided to include it later when it's fixed so we can focus on the Plugin System first.

Good idea! We'll see it coming.

> In chatmaster.h there are these functions that were originaly private:
> // Get all chats in all chat windows
> QList<Chat*>       getAllChats();
> // Return the window for the given chat
> ChatWindow        *getChatWindow( Chat *chat );
> // Return the chat window where we're having an conversation with the given contact.
> Chat              *getContactsChat( const QString &handle, bool privateChat );
> // Check whether the contact is in any of the existing chat windows.
> bool               isContactInChat( const QString &handle );
> 
> 
> I made them public as they are the only functions needed in kmesschat.cpp that weren't accesible, but what should  be better here (for the plugsys purpose)? make KMessChat a friend class of ChatMaster or leave it as I did making public only the necessary functions?

I think it's fine to make these functions public. They don't return any real private information, I think. Friend classes are usually something to avoid in situations like this, or you end up friend classing every class that needs a tiny bit of private information, that could very well be public, too.

> Also I'm not sure if this is a problem with my distro or so, but it seems that all the plugins are enabled by default when you get to the enable/disable window (they are configured to be disabled by default, though), I'll appreciate if somebody can confirm this.

I'll read through the patch tomorrow as soon as I can - if it looks OK I'll apply it locally and check if I have that behaviour too. Will let you know.

> In addition, KMess should create a 'pluginsconfig' folder on the account folder at install: .kde/share/apps/kmess-dev/em...@em.../pluginsconfig/plugin-folder-here
> How can this be done? Somewhere in CMakeLists.txt?

This is user-specific data (since you're talking about a .kde directory), and during installation, you never install user-specific data, only system-specific data. Remember that if I compile and system-wide-install kmess as myself, then another user on the system may want to use it too. The normal way to do this, is to create the directory during startup / initialisation, or whenever it is actually needed. You can use our existing configuration classes, or KDE's classes, to get to know what the KDE directory is (it's not always ~/.kde), and you can use QDir (iirc) to create the directory so you can create files in it.

As said, I'll look at the patch tomorrow. Sorry again for responding this late, it's been a busy time for me too :(

Thanks again,
Sjors