<br><br><div><span class="gmail_quote">On 7/16/07, <b class="gmail_sendername">Aaron J. Seigo</b> <<a href="mailto:aseigo@kde.org">aseigo@kde.org</a>> wrote:</span><blockquote class="gmail_quote" style="margin-top: 0; margin-right: 0; margin-bottom: 0; margin-left: 0; margin-left: 0.80ex; border-left-color: #cccccc; border-left-width: 1px; border-left-style: solid; padding-left: 1ex">
On Monday 16 July 2007, Leo Franchi wrote:<br><br>comments/thoughts follow; it's a pretty big patch so i have a number of<br>comments, hope you don't mind:<br><br>- sticking to the coding style is appreciated; it's already messy in a few
<br>places due to the many hands effect =) e.g. there's no spaces between ()s and<br>the contents, so QString( "text/x-plasmoidservicename" ) should just be<br>QString("text/x-plasmoidservicename") .. minor nitpick, really, and i won't
<br>shoot anyone if there a few extra spaces here and there, but it would be nice<br>to follow things as closely as possible.</blockquote><div><br>sorry about that. i tried to figure out if there was a consistent coding convention (especially for spaces inside parenthesis) but it seemed like it was pretty even between
<br>spaces-between-parens and no spaces. as Amarok code uses spaces, that was my default. anyway, i will fix up these typographical things :) </div><br><br><blockquote class="gmail_quote" style="margin-top: 0; margin-right: 0; margin-bottom: 0; margin-left: 0; margin-left: 0.80ex; border-left-color: #cccccc; border-left-width: 1px; border-left-style: solid; padding-left: 1ex">
- instead of ThemePath perhaps ThemePrefix?<br><br>- why do you need the list of loaded applets from Corona (loadedApplets())?<br>(not saying it's not valid, just wondering what the use case is)</blockquote><div><br>this is so there is a way to clear the corona. as applets are started by the user, currently from the controlbox, the contextscene (corona subclass) needs to know what is on
<br>the corona itself so it can a) save the state b) clear it c) set next state. as the user plays a track, for example, the corona should clear the "home" state and display context-<br>relative information. this is all done in the subclassed ContextScene, but as the applet list is in the dptr, it needs access to it.
</div><br><br><blockquote class="gmail_quote" style="margin-top: 0; margin-right: 0; margin-bottom: 0; margin-left: 0; margin-left: 0.80ex; border-left-color: #cccccc; border-left-width: 1px; border-left-style: solid; padding-left: 1ex">
<br>i wonder if it doesn't make sense to just use one setter for all of these. we<br>could easily have a single setter/getter pair in plasma.cpp for a static<br>QString in the Plasma namespace that sets/gets the prefix. this can then be
<br>used whereever we do KTrader lookups, etc..<br><br>so in plasma we'd use Plasma (making Plasma/Applet, Plasma/DataEngine,<br>Plasma_foorc) and in amarok you might use Amarok (making Amarok/Applet and<br>amarok_foorc). the two static strings can be kept (in the Private class) but
<br>having just one method for this would be nice. this would bring the number of<br>calls to set up libplasma for use in other apps from 5 down to 2 (one for the<br>theme prefix and one for the applet prefix). if we use this same approach for
<br>the drag-and-drop mimetype, we could even get rid of one more setter.<br><br>the downside to this approach is that, while simpler to use, it means that<br>it's an all-or-nothing approach ... it wouldn't be possible to use
<br>DataEngines for plasma in amarok and vice versa as a side effect of setting<br>up separate applets.<br><br>hm ... for that matter, i'm really not sure why we need to namespace<br>DataEngines at all? they are only requested by applets so the user never sees
<br>them; we just need to be sure to avoid name collissions which should be<br>pretty easy to do. that removes the whole "namespacing applets affects<br>dataengines" issue with the above approach.</blockquote><br>
<blockquote class="gmail_quote" style="margin-top: 0; margin-right: 0; margin-bottom: 0; margin-left: 0; margin-left: 0.80ex; border-left-color: #cccccc; border-left-width: 1px; border-left-style: solid; padding-left: 1ex">
we'd probably want to add some convenience methods to Plasma:: as well for<br>the "final" products (applet service type, drag and drop mimetype, etc)<br><br>- in theme.cpp there is this:<br><br>- KConfig config( "plasmarc" );
<br>- KConfigGroup group( &config, "Theme" );<br>+ KConfigGroup group = KGlobal::config()->group( "Theme" );<br><br>this will break krunner which shares the theming with plasma to create a
<br>consistent look, which is really the whole point of desktoptheme. =)<br><br>perhaps this should be re-thought a bit.... if we have the prefix setter, we<br>could put the theme setting in its own group Theme-$Prefix? this would work
<br>for the time being until someone comes up with a good reason for why they<br>need their own svg theme but not applets, or vice versa </blockquote><br>ok, i don't really know much about krunner so i had no idea of the repercussions of that change :)
<br><br><blockquote class="gmail_quote" style="margin-top: 0; margin-right: 0; margin-bottom: 0; margin-left: 0; margin-left: 0.80ex; border-left-color: #cccccc; border-left-width: 1px; border-left-style: solid; padding-left: 1ex">
- speaking of Plasma::Theme i don't know if changing from desktoptheme/ is<br>really necessary. as long as the theme setting can be changed independantly,<br>there's no reason other theme sets couldn't be stored in desktoptheme/ as
<br>well, perhaps prefixed if need be with amarok- (or whatever). another setter<br>and qstring member killed!</blockquote><div><br>also a good idea :)<br><br>tomorrow i'll fix up the patch and look to doing things how you have suggested.
<br><br>leo </div><br><blockquote class="gmail_quote" style="margin-top: 0; margin-right: 0; margin-bottom: 0; margin-left: 0; margin-left: 0.80ex; border-left-color: #cccccc; border-left-width: 1px; border-left-style: solid; padding-left: 1ex">
--<br>Aaron J. Seigo<br>humru othro a kohnu se<br>GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43<br><br>Full time KDE developer sponsored by Trolltech (<a href="http://www.trolltech.com">http://www.trolltech.com
</a>)<br><br>_______________________________________________<br>Amarok-devel mailing list<br><a href="mailto:Amarok-devel@kde.org">Amarok-devel@kde.org</a><br><a href="https://mail.kde.org/mailman/listinfo/amarok-devel">https://mail.kde.org/mailman/listinfo/amarok-devel
</a><br><br><br></blockquote></div><br><br clear="all"><br>-- <br>______________________________________________________<br>Leo Franchi <a href="mailto:angel666@myrealbox.com">angel666@myrealbox.com</a>
<br>4305 Charlemagne Ct <a href="mailto:lfranchi@gmail.com">lfranchi@gmail.com</a> <br>Austin cell: (650) 704 3680<br>TX, USA home: (650) 329 0125