KIcon API replacement (Re: patches on kcolors)

[David Faure]

On Thursday 15 March 2012 19:53:36 Kevin Ottens wrote:
> On Saturday 10 March 2012 13:03:10 David Faure wrote:
> > On Saturday 10 March 2012 11:49:21 David Faure wrote:
> > > Maybe we want to make it a method of
> > > KIconLoader/KIconEngine instead?
> > 
> > KIconLoader::loadIcon is already taken (and returns a QPixmap), and
> > KIconEngine is an internal class.
> > 
> > So a new suggestion would be:
> > 
> > namespace KDE {
> > 
> >   QIcon loadIcon(const QString& iconName, KIconLoader* iconLoader = 0,
> >   const
> > 
> > QStringList& overlays = QStringList());
> > }
> 
> Looks good to me. We probably want also an overload without the iconLoader
> parameter I guess.

It default to 0, so I assume you mean for the case where someone wants to 
specify overlays and use the default iconloader, and a ,0 would look ugly?

> ***
> As a bonus, the crazy idea of the day:
> What about making an exception to the casing rule for naming in cases like
> that (not excluding there's more than K/QIcon matching that pattern) and
> going for:
> namespace KDE {
>     QIcon Icon(...);
> }
> 
> Client code would then look like:
> QIcon i = KDE::Icon("foo");
> instead of:
> QIcon i = KDE::loadIcon("foo");
> 
> (I personally think it conveys better the idea as it makes it feel almost
> like a prototype object)

Well, it's not so crazy. After I send the mail I thought, icon() would be 
better than loadIcon(), especially since the loading doesn't happen at that 
point, but on demand.
I was thinking icon() lowercase though, Icon() is a little bit crazier indeed.

But I'm not necessarily against crazy ideas ;-)

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
Sponsored by Nokia to work on KDE, incl. KDE Frameworks 5