KDEREVIEW: nowplaying dataengine and applet for plasma

[Alex Merry]

At Sunday 10 February 2008 01:30, you wrote:
> On Saturday 09 February 2008, Alex Merry wrote:
> > This has been the only comment (apart from the discussion of the merits and
> 
> this was posted to k-c-d while i was in an airplane (for the next 2 days, as 
> it turned out, with intermittent 'net access) so i missed it. it doesn't seem 
> to have been CC'd to panel-devel until just now .. =/

Sorry about that...

> 
> looking at the engine i have a few comments:
> 
> * none of the strings are i18n'd, and none of the keys are I18N_NOOP'd.

I had a long discussion on #kde-i18n about that...

Basically, without context, the strings are incredibly ambigious for translators.  After all, "artist" means something if you know it refers to a piece of music, but could have several meanings if you don't know that.  And if context is provided with I18N_NOOP2, identical context must be provided with the corresponding i18nc() call, which is an unreasonable burden on applet writers IMHO.

aacid was rather... lukewarm... to the idea that we were using translatable strings as keys.

> 
> * providing a "help" source is interesting. perhaps we should make that 
> standard, though really ... that's for people designing widgets, isn't it? 
> it's not particularly for the actual usage of the engine. documentation is an 
> unanswered question with engines, and it would be good to provide a better 
> way to do this that doesn't involve having to open up khelpcenter either.

That's on my TODO list for 4.1.  For now, it provides a way of people getting the doc without reading the source.  Once we have a better way, I'll probably scrap the "help" and "properties" sources.

> 
> * using DataEngine for a command sink is actually rather wrong from the design 
> perspective of DataEngine. i'm sure it works, but it's really not what Engine 
> was designed for. there's no feedback on success, for instance. we discussed 
> this exact issue in the plasma meeting Saturday and we decided to provide a 
> more appropriate interface for commands as it's a common need. but DataEngine 
> is designed to provide access to data, in a one-to-many relationship which 
> makes these things not so fun to do.

Yes, but we only had the meeting yesterday :-P

As soon as we have a DataService class, that will go.

> 
> on to the applet:
> 
>     dataEngine("nowplaying")->connectSource("Players", this, 5000);
> 
> should the players source update itself? polling is Bad(tm). what should 
> probably be happening in the engine is in addPlayer, the Player source should 
> be being updated. that is constant data that is always available and pretty 
> much required, after all. then you can get rid of the 5s poll above and just 
> connect to the source normally.

Point.  Will change, although addPlayer() isn't where it should be (Juk and XMMS are added using addPlayer() even if they don't exist, because the object needs to be there to check whether the player exists...)

> 
>     connect(this, SIGNAL(geometryChanged()), this, SLOT(sizeChanged()));
> 
> what you probably really want to do is respond to the SizeConstraint in 
> constriantsUpdated. you also don't want to call updateGeometry() 
> indiscriminently in constraintsUpdated(). in fact, updateGeometry is called 
> for you when needed before constraintsUpdated() is called; the only time the 
> applet should need to call it is when it changes its own geometry.

OK.  The API docs really need more work.  *notes that this is on his own TODO list for 4.1*

I got a little confused by what was and wasn't required or possible in constraintsUpdated().

Alex