----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1195/#review1873 -----------------------------------------------------------
erg.. i keep forgetting to review this .. a couple comments follow. maybe we can get together next week in person and do a full code review? /trunk/KDE/kdelibs/plasma/containment.cpp <http://reviewboard.kde.org/r/1195/#comment1220> this will need to move into Corona as it is very application specific. perhaps the idea of a "global" set would make sense, and if there are no containment-specific settings it would use the global set. this would make configuring all containments easier while allowing one to override them on a per-containment basis if needed/desired. /trunk/KDE/kdelibs/plasma/containment.cpp <http://reviewboard.kde.org/r/1195/#comment1221> keys() is slow. use a proper iterator. will get rid of the call to value() on the next line as well. /trunk/KDE/kdelibs/plasma/containment.cpp <http://reviewboard.kde.org/r/1195/#comment1222> cute comment ;) - Aaron On 2009-08-01 00:49:47, Chani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/1195/ > ----------------------------------------------------------- > > (Updated 2009-08-01 00:49:47) > > > Review request for Plasma. > > > Summary > ------- > > this is the first of three patches I've finally persuaded reviewboard to > accept. > it contains all my libplasma changes for my gsoc project. > > I've created a ContextAction class, based on Wallpaper, made Containment keep > some ContextActions, and added some API there to support it. > I got some API review at akademy, but I've changed API a bit since then too, > and I'm sure there are things I've missed in hte code... > for more details see > http://gitorious.org/plasma-mouse-plugins/kdelibs-plasma/commits/gsoc and > other branches. gsoc has the squashed commits, and master is just trunk with > none of my changes. > > known issues: > -I haven't created one of those include thingies in kdelibs yet because it'd > be outside my gitsvn repo, so the #include stuff in workspace has to use the > filename. I'll fix that once this is in trunk. > -while it's sorta possible internally to use the same plugin on two different > triggers, the plugin's config will be shared and it'll confuse the config UI. > I'm not going to support multiple plugin instances unless someone persuades > me it's really useful and i have time later. > -plugins are per-containment. the advantage is you can have different plugins > on a different activity (eg. a different set of program launchers). the > disadvantage is it's tedious to set up something the same on all activities. > -rightclicking applethandles doesn't work ATM. I'd like to fix that so it's > the same as rightclicking the applet, but it's not a high priority. > -recently I've been having trouble with folderview as a containment; > rightclick doesn't work and leftclick works but dismisses the dashboard. > after this is merged I'll have to go look into what folderview is doing with > mouse events... > -I'm not sure if the mouse events in Containment are doing hte right thing > when !isContainment(). everything appears to work smoothly, but the code has > become a bit.. strange. > > > Diffs > ----- > > /trunk/KDE/kdelibs/plasma/CMakeLists.txt 1005300 > /trunk/KDE/kdelibs/plasma/applet.h 1005300 > /trunk/KDE/kdelibs/plasma/applet.cpp 1005300 > /trunk/KDE/kdelibs/plasma/containment.h 1005300 > /trunk/KDE/kdelibs/plasma/containment.cpp 1005300 > /trunk/KDE/kdelibs/plasma/contextaction.h PRE-CREATION > /trunk/KDE/kdelibs/plasma/contextaction.cpp PRE-CREATION > /trunk/KDE/kdelibs/plasma/corona.cpp 1005300 > /trunk/KDE/kdelibs/plasma/private/containment_p.h 1005300 > /trunk/KDE/kdelibs/plasma/private/contextaction_p.h PRE-CREATION > /trunk/KDE/kdelibs/plasma/private/packages.cpp 1005300 > /trunk/KDE/kdelibs/plasma/private/packages_p.h 1005300 > /trunk/KDE/kdelibs/plasma/servicetypes/plasma-contextaction.desktop > PRE-CREATION > > Diff: http://reviewboard.kde.org/r/1195/diff > > > Testing > ------- > > > Thanks, > > Chani > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
