Consideration requested for merging of kdelibs-action branch

[Simon Hausmann]

On Monday 13 March 2006 22:33, David Faure wrote:
> Hi Hamish,
>
> great work on the branch, I'm convinced that it should be merged in.
>
> Just one comment on a possible future change:
>
> On Sunday 12 March 2006 16:37, Hamish Rodda wrote:
> > * Fix the issue with icon loading:
> >
> > "The icon name we should be able to solve nicely through QIconEngine.
> > Instead of passing the QString iconName around we can pass around a QIcon
> > that was constructed with an internal KIconEngine. Then we call addFile
> > on it so that the icon engine can remember it. That's it, from then on we
> > can pass the QIcon around and whenever someone needs the actual pixmap he
> > can request it through QIcon or (more likely) call QIcon's paint method
> > directly. That way we can also implement svg icons."
>
> At the KAction API level, I am rather against this change. Being able to
> pass a string with the icon name when constructing the action, means that
> the app code is completely independent from the underlying technologies
> used to load the icon. new KAction( ... "reload" ... ) will always be
> valid, whichever image format is used, whichever icon theme mechanism is in
> place, whichever Qt or KDE class has to be used to load the icon, etc.
>
> On the other hand, having to write
>   QIcon reloadIcon( KIconEngine::self() );   // or KGlobal::iconEngine() or
> something reloadIcon.addFile( "reload" ); // or worse,
> KGlobal::dirs()->...("reload") .... new KAction( ... reloadIcon ... )
> would be a major PITA, obviously. Number of lines gets multiplied by 3,
> with lots of technology-dependent code added.
>
> OK this is obvious enough so that it's likely I just misunderstood your
> paragraph; but let me send this mail to make sure it's the case, and that
> passing anything else than a QString at the API level isn't even considered
> :)

Yes, manual construction like that is certainly not an option. But on the API 
level a pure QString as handle isn't very useful either once you want to 
actually do something with it.

That's why I like a compromise where a QString is used for construction/setup 
but the object you actually pass around and query is a QIcon, because unlike 
QString it offers an API optimized for icons, in particular a simple paint() 
method that takes a QPainter, a rect and a few options. That's much easier 
than providing a QString and then requiring that the developer has to look up 
some global function somewhere where he can pass in the string along with a 
painter and a rect.

The piece of documentation that tells the developer that 'filesave' is the 
icon to use when it comes to saving all sorts of documents could also be the 
place that says: 'Use KIcon::saveFile()' or 'KIcon::fromName("filesave")', 
for example.

Of course there could also be a KIcon constructor taking a QString, but given 
that it would behave quite different to the corresponding QIcon constructor 
I'm not sure if that would be a good idea.

Simon