Consideration requested for merging of kdelibs-action branch

David Faure faure at kde.org
Mon Mar 13 21:33:13 GMT 2006


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 :)

-- 
David Faure, faure at kde.org, sponsored by Trolltech to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).





More information about the kde-core-devel mailing list