Re: Review Request: allow SVGs to use systemcolors
--- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5495/ --- (Updated 2010-10-01 08:19:06.216337) Review request for Plasma. Changes --- don't change the id of the style-tag Summary --- With this patch applied SVGs can put a style-element with id 'current-system-colors' in it, which gets replaced by a style with the current systemcolors. This allows SVGs to use colors like background color and text color from the system palette. Giving themes much more possibilitys then just coloring the resulting pixmap. Diffs (updated) - /trunk/KDE/kdelibs/plasma/theme.h 1180314 /trunk/KDE/kdelibs/plasma/theme.cpp 1180314 /trunk/KDE/kdelibs/plasma/svg.cpp 1180314 Diff: http://svn.reviewboard.kde.org/r/5495/diff Testing --- Changing theme, changing colorscheme Thanks, Manuel ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Adding Set Wallpaper Option In Frame Applet
So no one has a suggestion, or am I doing something terribly wrong? On 9/30/10, Sinny Kumari ksi...@gmail.com wrote: Hi, I want to add Set Wallpaper option in Frame Applet. while doing this, I found one option in directory trunk/kde/src/KDE/kdelibs/plasma/ in the containment.cpp file which emits signal and sets the wallpaper when an image is dropped on the Desktop while image option is selected. That line is emit wallpaper-urlDropped(tjob-url()); which is defined in the function void ContainmentPrivate::mimeTypeRetrieved(KIO::Job *job, const QString mimetype) and class ContainmentPrivate is in trunk/kde/src/KDE/kdelibs/plasma/private. I want to implement it in Frame applet, but, right now I don't know, how can I implement the same thing in Frame applet which is in kdeplasma-addons/applet/frame. So, If anybody have some idea that how it can be done then, please share with me. Thanks! -- http://www.sinny.in -- http://www.sinny.in ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: allow SVGs to use systemcolors
--- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5495/ --- (Updated 2010-10-01 14:25:03.138571) Review request for Plasma. Summary --- With this patch applied SVGs can put a style-element with id 'current-system-colors' in it, which gets replaced by a style with the current systemcolors. This allows SVGs to use colors like background color and text color from the system palette. Giving themes much more possibilitys then just coloring the resulting pixmap. Diffs - /trunk/KDE/kdelibs/plasma/theme.h 1180314 /trunk/KDE/kdelibs/plasma/theme.cpp 1180314 /trunk/KDE/kdelibs/plasma/svg.cpp 1180314 Diff: http://svn.reviewboard.kde.org/r/5495/diff Testing --- Changing theme, changing colorscheme Screenshots (updated) --- Default Colorscheme http://svn.reviewboard.kde.org/r/5495/s/520/ green-gray Colorscheme http://svn.reviewboard.kde.org/r/5495/s/522/ Thanks, Manuel ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: allow SVGs to use systemcolors
On 2010-09-30 13:10:03, Marco Martin wrote: /trunk/KDE/kdelibs/plasma/svg.cpp, line 441 http://svn.reviewboard.kde.org/r/5495/diff/1/?file=38739#file38739line441 so the coloring of the pixmap is still kept as retrocompatibility? Manuel Mommertz wrote: I don't want to break existing themes ;) agree. On 2010-09-30 13:10:03, Marco Martin wrote: /trunk/KDE/kdelibs/plasma/theme.cpp, line 790 http://svn.reviewboard.kde.org/r/5495/diff/1/?file=38741#file38741line790 the button widget should be modified if this will start to be used Manuel Mommertz wrote: For what reason? i see on hover events the pushbutton searches for an active element (not even used anymore by the default theme btw), so if that element uses ButtonHoverColor should already e possible to color it On 2010-09-30 13:10:03, Marco Martin wrote: /trunk/KDE/kdelibs/plasma/theme.cpp, line 320 http://svn.reviewboard.kde.org/r/5495/diff/1/?file=38741#file38741line320 i wonder how much is the benefit vs cost of caching this? Manuel Mommertz wrote: Both is quite low. ;) But as this stylesheet has to be generated for every Plasma::Svg object, which is 259 times for a start of plasma with the default setup + analog clock + calculator, it feels wrong to regenerate it that often. ok - Marco --- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5495/#review7905 --- On 2010-10-01 14:25:03, Manuel Mommertz wrote: --- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5495/ --- (Updated 2010-10-01 14:25:03) Review request for Plasma. Summary --- With this patch applied SVGs can put a style-element with id 'current-system-colors' in it, which gets replaced by a style with the current systemcolors. This allows SVGs to use colors like background color and text color from the system palette. Giving themes much more possibilitys then just coloring the resulting pixmap. Diffs - /trunk/KDE/kdelibs/plasma/theme.h 1180314 /trunk/KDE/kdelibs/plasma/theme.cpp 1180314 /trunk/KDE/kdelibs/plasma/svg.cpp 1180314 Diff: http://svn.reviewboard.kde.org/r/5495/diff Testing --- Changing theme, changing colorscheme Screenshots --- Default Colorscheme http://svn.reviewboard.kde.org/r/5495/s/520/ green-gray Colorscheme http://svn.reviewboard.kde.org/r/5495/s/522/ Thanks, Manuel ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: allow SVGs to use systemcolors
On 2010-09-30 13:10:03, Marco Martin wrote: /trunk/KDE/kdelibs/plasma/svg.cpp, line 107 http://svn.reviewboard.kde.org/r/5495/diff/1/?file=38739#file38739line107 a check elements with the same id aren't existing should be done Manuel Mommertz wrote: You are right. But this would add more clutter. Maybe it is better to just leave the id as is. So no hint-uses-color-scheme but only current-color-scheme. so should be specified in the theme documentation that it is reserved - Marco --- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5495/#review7905 --- On 2010-10-01 14:25:03, Manuel Mommertz wrote: --- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5495/ --- (Updated 2010-10-01 14:25:03) Review request for Plasma. Summary --- With this patch applied SVGs can put a style-element with id 'current-system-colors' in it, which gets replaced by a style with the current systemcolors. This allows SVGs to use colors like background color and text color from the system palette. Giving themes much more possibilitys then just coloring the resulting pixmap. Diffs - /trunk/KDE/kdelibs/plasma/theme.h 1180314 /trunk/KDE/kdelibs/plasma/theme.cpp 1180314 /trunk/KDE/kdelibs/plasma/svg.cpp 1180314 Diff: http://svn.reviewboard.kde.org/r/5495/diff Testing --- Changing theme, changing colorscheme Screenshots --- Default Colorscheme http://svn.reviewboard.kde.org/r/5495/s/520/ green-gray Colorscheme http://svn.reviewboard.kde.org/r/5495/s/522/ Thanks, Manuel ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: allow SVGs to use systemcolors
--- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5495/#review7925 --- Ship it! i think it's pretty good now - Marco On 2010-10-01 14:25:03, Manuel Mommertz wrote: --- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5495/ --- (Updated 2010-10-01 14:25:03) Review request for Plasma. Summary --- With this patch applied SVGs can put a style-element with id 'current-system-colors' in it, which gets replaced by a style with the current systemcolors. This allows SVGs to use colors like background color and text color from the system palette. Giving themes much more possibilitys then just coloring the resulting pixmap. Diffs - /trunk/KDE/kdelibs/plasma/theme.h 1180314 /trunk/KDE/kdelibs/plasma/theme.cpp 1180314 /trunk/KDE/kdelibs/plasma/svg.cpp 1180314 Diff: http://svn.reviewboard.kde.org/r/5495/diff Testing --- Changing theme, changing colorscheme Screenshots --- Default Colorscheme http://svn.reviewboard.kde.org/r/5495/s/520/ green-gray Colorscheme http://svn.reviewboard.kde.org/r/5495/s/522/ Thanks, Manuel ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request: use sqlite for storage
--- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5506/ --- Review request for Plasma. Summary --- looking at how slow kconfig as, this makes storage use sqlite (and makes some methods private, before it's too late) it can still use some improvements but it's basically working. two main concerns are: - is acceptable/safe to link to QtSql and assume the sqlite driver is present? - i would still see this as a fallback for when an akonadi version is not present (being in another process should slowdown the gui a bit less, but i could not want it in some mobile profiles) the akonadi version is in the usual status almost working with developer disappeared :p Diffs - /trunk/KDE/kdelibs/plasma/CMakeLists.txt 1179394 /trunk/KDE/kdelibs/plasma/datacontainer.h 1179394 /trunk/KDE/kdelibs/plasma/datacontainer.cpp 1179394 /trunk/KDE/kdelibs/plasma/dataengine.cpp 1179394 /trunk/KDE/kdelibs/plasma/private/datacontainer_p.h 1179394 /trunk/KDE/kdelibs/plasma/private/storage.cpp 1179394 /trunk/KDE/kdelibs/plasma/private/storage_p.h 1179394 Diff: http://svn.reviewboard.kde.org/r/5506/diff Testing --- Thanks, Marco ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Adding Set Wallpaper Option In Frame Applet
On Friday, October 1, 2010, Sinny Kumari wrote: So no one has a suggestion, or am I doing something terribly wrong? sorry, just a very busy week for me and things are falling through the cracks. thanks for bringing this thread back to life :) On 9/30/10, Sinny Kumari ksi...@gmail.com wrote: and class ContainmentPrivate is in trunk/kde/src/KDE/kdelibs/plasma/private. I want to implement it in Frame applet, but, right now I don't know, how can I implement the same thing in Frame applet which is in kdeplasma-addons/applet/frame. how the wallpaper URL is stored in the config is up to the Wallpaper plugin. i don't think we can change that without putting all kinds of requirements on the wallpaper itself. basically, we need to know three things from the containment: * does it support wallpapers at all? (Containment::setDrawWallpaper()) if not, don't show this feature in Frame * does it have a wallpaper currently? (Containment::wallpaper()) if not, then simply setting the wallpaper to be the Image paper or just not offering the opeiont at all from Fram would be the answer. * does the wallpaper support random images? we have nothing for this right now. if the wallpaper doesn't, then Frame shouldn't show this option. if all of the above is met, then we need a way to add an image to a wallpaper. there is no way to do this currently, either. so to make this happen, we probably need to extend Plasma::Wallpaper. in particular: * it should have a property noting whether or not it is image file based. this can be set by the plugin. * an addWallpaper(const KUrl url) method or something like this. due to binary compat issues, it can't be virtual. but it can be public and it can either: QMetaObject::invoke a protected slot, which plugins can over-ride, or it can emit a signal which plugins would then need to connect to. i'd prefer the protected slot, because then when we break BC we can just make that method virtual public and not touch the plugins. -- Aaron J. Seigo humru othro a kohnu se GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43 KDE core developer sponsored by Qt Development Frameworks signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: use sqlite for storage
--- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5506/#review7926 --- the implementation looks good; my one question is regards keeping the data from different applets separate. /trunk/KDE/kdelibs/plasma/private/storage.cpp http://svn.reviewboard.kde.org/r/5506/#comment8176 instead of an incrementing connectionId, you could also use the value of the this pointer? one less static int kicking around. /trunk/KDE/kdelibs/plasma/private/storage.cpp http://svn.reviewboard.kde.org/r/5506/#comment8178 destination() is no longer used; with one db file used for all the storage, we end up with all the data in one db with no separation. it would make sense to me here to create a table for each destination(). on applet destruction, it could drop its table (if any). on applet export, it could pull its table over to an exported db. (i imagine that Corona would use this in its export method). /trunk/KDE/kdelibs/plasma/private/storage.cpp http://svn.reviewboard.kde.org/r/5506/#comment8179 ah, sql databases. this query will result in multiple rows with the same key, and when requested back you'll end up with a random row. to fix this, first delete the old row. or check if the old row exists and do an update instead of an insert. and no, there is no update or insert in sql. that's what sorted procedures are for ;P - Aaron On 2010-10-01 15:05:11, Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5506/ --- (Updated 2010-10-01 15:05:11) Review request for Plasma. Summary --- looking at how slow kconfig as, this makes storage use sqlite (and makes some methods private, before it's too late) it can still use some improvements but it's basically working. two main concerns are: - is acceptable/safe to link to QtSql and assume the sqlite driver is present? - i would still see this as a fallback for when an akonadi version is not present (being in another process should slowdown the gui a bit less, but i could not want it in some mobile profiles) the akonadi version is in the usual status almost working with developer disappeared :p Diffs - /trunk/KDE/kdelibs/plasma/CMakeLists.txt 1179394 /trunk/KDE/kdelibs/plasma/datacontainer.h 1179394 /trunk/KDE/kdelibs/plasma/datacontainer.cpp 1179394 /trunk/KDE/kdelibs/plasma/dataengine.cpp 1179394 /trunk/KDE/kdelibs/plasma/private/datacontainer_p.h 1179394 /trunk/KDE/kdelibs/plasma/private/storage.cpp 1179394 /trunk/KDE/kdelibs/plasma/private/storage_p.h 1179394 Diff: http://svn.reviewboard.kde.org/r/5506/diff Testing --- Thanks, Marco ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Fix PaintUtils::shadowText() placement
--- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5504/#review7927 --- Ship it! - Aaron On 2010-10-01 02:00:06, Christoph Feck wrote: --- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5504/ --- (Updated 2010-10-01 02:00:06) Review request for Plasma. Summary --- I used Plasma::PaintUtils::shadowText() to draw a text halo (offset 0,0) around a text. Unfortunately, that halo got cropped (see the g). This patch should * fix placement of shadow and text inside the resulting pixmap * fix addSize not enlarged for negative offsets * simplify computation Diffs - /trunk/KDE/kdelibs/plasma/paintutils.cpp 1181424 Diff: http://svn.reviewboard.kde.org/r/5504/diff Testing --- Screenshots --- before http://svn.reviewboard.kde.org/r/5504/s/517/ after http://svn.reviewboard.kde.org/r/5504/s/519/ Thanks, Christoph ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: bug fixes for the system-monitor applet
On 2010-09-25 08:31:36, Petri Damstén wrote: Looks good to me. has it been committed? if not, can you please do so, and mark this as submitted. thanks. - Aaron --- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/3950/#review7773 --- On 2010-05-14 08:21:03, Michel Lafon-Puyo wrote: --- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/3950/ --- (Updated 2010-05-14 08:21:03) Review request for Plasma. Summary --- As my first work for KDE, I would like to add some power management monitoring features to the system-monitor applet. Monitoring the cpu clock frequency when using the on-demand or the conservative governor could be useful (at least it is for me :)). Before going further in this development I tried to fix some little bugs with the applet. Here is the list of the changes this patch introduces: - the size of the applets when used in the panel and when no item is monitored was (0,0) but an icon were displayed and it overlapped the other applets (SM::Applet::CheckGeometry()) - set the preferred height to MINIMUM when no item is monitored (SM::Applet::displayNoAvailableSources()). When all the meters of an applet were removed at once, the size were unchanged and a big icon could be displayed. - clear the content of the tooltip when nothing has to be displayed (SM::Applet::toolTipAboutToShow()) - when one or many items were set unmonitored, the corresponding widgets were not correctly deleted (SM::Applet::deleteMeters()) - the removal of the layout and the meters were done on SM::Applet::connectToEngine(), I moved that part in a new method SM::Applet::removeLayout() that can be called more easily. This method is called by the applets on configuration change to achieve a clean update. - the header of the applets is now correctly deleted on form factor change and should not be displayed when the applet is used in the panel - when used in the panel, the webview containing the information given by the hardware information applet was displayed under the icon because the webview and the icon were not correctly deleted on form factor changes. - to be consistent with the other applets, the HDD applet has been changed to *not* populate the configuration with the list of the mounted volumes when there is no more item to monitor. Nevertheless, on the first launch (no configuration is present), the behaviour has not changed and the configuration is still populated with the list of the mounted volumes. Additionnally, the HDD applet didn't use the SM::Applet to manage its meters. So I replaced the list of SM::Plotter (m_plotters) by a list of QGraphicsWidget (m_meters) and modified HDD to take advantage of the meters management already implemented in the SM::Applet class (in particular the removal of the meters/widgets on configuration change as mentioned above). Note: I was unable to find any bug reports corresponding to the fixes above. Should I create them myself? Diffs - /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/applet.h 1126529 /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/applet.cpp 1126529 /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/cpu.h 1126529 /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/cpu.cpp 1126529 /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/hdd.h 1126529 /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/hdd.cpp 1126529 /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/hwinfo.h 1126529 /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/hwinfo.cpp 1126529 /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/net.h 1126529 /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/net.cpp 1126529 /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/ram.h 1126529 /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/ram.cpp 1126529 /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/temperature.h 1126529 /trunk/KDE/kdebase/workspace/plasma/generic/applets/system-monitor/temperature.cpp 1126529 Diff: http://svn.reviewboard.kde.org/r/3950/diff Testing --- Basic testing Thanks, Michel ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Do not use tree view for categories in the vertical widgets explorer
--- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5446/#review7929 --- Ship it! much nicer. - Aaron On 2010-09-27 10:57:33, Anselmo L. S. Melo wrote: --- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5446/ --- (Updated 2010-09-27 10:57:33) Review request for Plasma. Summary --- The propose here is to remove the Tree View used in the widgets explorer in vertical orientation. In its place, use the same button of the horizontal orientation, what simplifies the code (removes a class and some checks) and improves the use of space as the widgets list can grow vertically. Better if combined with http://reviewboard.kde.org/r/4676/ as we also gain the space currently occupied by the arrow buttons and the close button in the bottom left corner. The screenshots attached show the current widget explorer with the categories tree view and the proposed one, using the categories button. Diffs - /trunk/KDE/kdebase/workspace/libs/plasmagenericshell/widgetsexplorer/appletsfiltering.h 1179288 /trunk/KDE/kdebase/workspace/libs/plasmagenericshell/widgetsexplorer/appletsfiltering.cpp 1179288 Diff: http://svn.reviewboard.kde.org/r/5446/diff Testing --- Tested in both horizontal and vertical orientations. Screenshots --- current widgets explorer http://svn.reviewboard.kde.org/r/5446/s/511/ proposed, not using the tree view http://svn.reviewboard.kde.org/r/5446/s/512/ Thanks, Anselmo ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Use Plasma::ScrollWidget in the widget explorer
--- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/4676/#review7930 --- Ship it! update to current trunk if needed, and let's get this in as well. - Aaron On 2010-08-04 21:02:11, Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/4676/ --- (Updated 2010-08-04 21:02:11) Review request for Plasma. Summary --- Make AbstractIconList inherit from Plasma::ScrollWidget, has discussed on plasma-devel. The horizontal orientation behaved a bit strangely: AbstractIconList was becoming much larger than the screen width. I had to change the layout code to include the Close button inside FilteringWidget layout instead of creating another layout. Note: you need http://reviewboard.kde.org/r/4675/ to get proper scrollbar slider sizes. Diffs - trunk/KDE/kdebase/workspace/libs/plasmagenericshell/abstracticonlist.h 1147944 trunk/KDE/kdebase/workspace/libs/plasmagenericshell/abstracticonlist.cpp 1147944 trunk/KDE/kdebase/workspace/libs/plasmagenericshell/widgetsexplorer/appletsfiltering.h 1147944 trunk/KDE/kdebase/workspace/libs/plasmagenericshell/widgetsexplorer/appletsfiltering.cpp 1147944 trunk/KDE/kdebase/workspace/libs/plasmagenericshell/widgetsexplorer/appletslist.h 1147944 trunk/KDE/kdebase/workspace/libs/plasmagenericshell/widgetsexplorer/appletslist.cpp 1147944 trunk/KDE/kdebase/workspace/libs/plasmagenericshell/widgetsexplorer/widgetexplorer.cpp 1147944 Diff: http://svn.reviewboard.kde.org/r/4676/diff Testing --- Tested in both horizontal and vertical modes, with lists larger and smaller than the view. Thanks, Aurélien ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: use sqlite for storage
On 2010-10-01 17:13:46, Aaron Seigo wrote: /trunk/KDE/kdelibs/plasma/private/storage.cpp, line 41 http://svn.reviewboard.kde.org/r/5506/diff/1/?file=38787#file38787line41 instead of an incrementing connectionId, you could also use the value of the this pointer? one less static int kicking around. uhm, i tought using a pointer value was even uglier, but ok. another thing that i tought seeing it. is really necessary having a different connection per job, since the query execution is afaik syncronous? even tought aboout a little singleton that holds a single QSqlDatabase... On 2010-10-01 17:13:46, Aaron Seigo wrote: /trunk/KDE/kdelibs/plasma/private/storage.cpp, lines 46-49 http://svn.reviewboard.kde.org/r/5506/diff/1/?file=38787#file38787line46 destination() is no longer used; with one db file used for all the storage, we end up with all the data in one db with no separation. it would make sense to me here to create a table for each destination(). on applet destruction, it could drop its table (if any). on applet export, it could pull its table over to an exported db. (i imagine that Corona would use this in its export method). or, destination could be a field (with key,destination being the primary key) would be ugly, but easier to expire old entries but yeah, i like more the different tbles approach in general also, if this is going to be used by applets, the source field doesn't make sense here... On 2010-10-01 17:13:46, Aaron Seigo wrote: /trunk/KDE/kdelibs/plasma/private/storage.cpp, line 66 http://svn.reviewboard.kde.org/r/5506/diff/1/?file=38787#file38787line66 ah, sql databases. this query will result in multiple rows with the same key, and when requested back you'll end up with a random row. to fix this, first delete the old row. or check if the old row exists and do an update instead of an insert. and no, there is no update or insert in sql. that's what sorted procedures are for ;P uhm, i think even worse, since key is primary the inseret should just silently fail, so the old stale entry wins i think i'll just try to delete the line before. - Marco --- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5506/#review7926 --- On 2010-10-01 15:05:11, Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5506/ --- (Updated 2010-10-01 15:05:11) Review request for Plasma. Summary --- looking at how slow kconfig as, this makes storage use sqlite (and makes some methods private, before it's too late) it can still use some improvements but it's basically working. two main concerns are: - is acceptable/safe to link to QtSql and assume the sqlite driver is present? - i would still see this as a fallback for when an akonadi version is not present (being in another process should slowdown the gui a bit less, but i could not want it in some mobile profiles) the akonadi version is in the usual status almost working with developer disappeared :p Diffs - /trunk/KDE/kdelibs/plasma/CMakeLists.txt 1179394 /trunk/KDE/kdelibs/plasma/datacontainer.h 1179394 /trunk/KDE/kdelibs/plasma/datacontainer.cpp 1179394 /trunk/KDE/kdelibs/plasma/dataengine.cpp 1179394 /trunk/KDE/kdelibs/plasma/private/datacontainer_p.h 1179394 /trunk/KDE/kdelibs/plasma/private/storage.cpp 1179394 /trunk/KDE/kdelibs/plasma/private/storage_p.h 1179394 Diff: http://svn.reviewboard.kde.org/r/5506/diff Testing --- Thanks, Marco ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request: Optional passive calendar popup for the Digital Clock
--- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5513/ --- Review request for Plasma. Summary --- I know some people don't like this idea thus the default is the same as it's right now. I want to make the calendar passive popup optional since some people like it that way (me included). OK to commit? Diffs - /trunk/KDE/kdebase/workspace/libs/plasmaclock/clockapplet.cpp 1181669 /trunk/KDE/kdebase/workspace/plasma/generic/applets/digital-clock/clock.cpp 1181669 Diff: http://svn.reviewboard.kde.org/r/5513/diff Testing --- Tested it a few times with true and false and seems to work just fine. The only downside is that you either have to relog or somehow reload that digital-clock plasmoid when you change the passivePopup value. Thanks, Mark ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Optional passive calendar popup for the Digital Clock
--- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5513/#review7934 --- as discussed earlier today on irc, and for all the reasons discussed there, this is not a change we want or care to have. it's one more code path that will end up being maintained by yours truly in the future. sorry. you are completely welcome to maintain this as a separate plasmoid or as a patch to the source, of course. The only downside is that you either have to relog or somehow reload that digital-clock plasmoid when you change the passivePopup value. that's probably because you need to read the value in configChanged(). /trunk/KDE/kdebase/workspace/libs/plasmaclock/clockapplet.cpp http://svn.reviewboard.kde.org/r/5513/#comment8183 i don't see why the behaviour should be different in other form factors. *shrug* - Aaron On 2010-10-02 02:05:02, Mark wrote: --- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5513/ --- (Updated 2010-10-02 02:05:02) Review request for Plasma. Summary --- I know some people don't like this idea thus the default is the same as it's right now. I want to make the calendar passive popup optional since some people like it that way (me included). OK to commit? Diffs - /trunk/KDE/kdebase/workspace/libs/plasmaclock/clockapplet.cpp 1181669 /trunk/KDE/kdebase/workspace/plasma/generic/applets/digital-clock/clock.cpp 1181669 Diff: http://svn.reviewboard.kde.org/r/5513/diff Testing --- Tested it a few times with true and false and seems to work just fine. The only downside is that you either have to relog or somehow reload that digital-clock plasmoid when you change the passivePopup value. Thanks, Mark ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
