----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.vidsolbach.de/r/163/#review142 -----------------------------------------------------------
there are a few issues to do with setting the size in init() for instance, but other than that (and a handful of style issues) it looks ok. the configuration dialog really needs some love though. trunk/KDE/kdeplasma-addons/applets/fileWatcher/fileWatcher.cpp <http://reviewboard.vidsolbach.de/r/163/#comment107> should be each on their own line: : Plasma::Applet(parent, args), m_configDialog(NULL), m_autoResize(false) trunk/KDE/kdeplasma-addons/applets/fileWatcher/fileWatcher.cpp <http://reviewboard.vidsolbach.de/r/163/#comment110> spaces around the '=' trunk/KDE/kdeplasma-addons/applets/fileWatcher/fileWatcher.cpp <http://reviewboard.vidsolbach.de/r/163/#comment108> needs braces {}; yes, even on one liners =) trunk/KDE/kdeplasma-addons/applets/fileWatcher/fileWatcher.cpp <http://reviewboard.vidsolbach.de/r/163/#comment109> never resize in init(), as that will clobber user defined size on restore. instead, set a reasonable default size in the constructor (e.g. resize(400, 400)) and then in init, if (m_autoResize) { // change it trunk/KDE/kdeplasma-addons/applets/fileWatcher/fileWatcher.cpp <http://reviewboard.vidsolbach.de/r/163/#comment111> braces, spaces around != trunk/KDE/kdeplasma-addons/applets/fileWatcher/fileWatcher.cpp <http://reviewboard.vidsolbach.de/r/163/#comment112> spaces around == trunk/KDE/kdeplasma-addons/applets/fileWatcher/fileWatcher.cpp <http://reviewboard.vidsolbach.de/r/163/#comment113> do you really want to resize the height as well? trunk/KDE/kdeplasma-addons/applets/fileWatcher/fileWatcher.cpp <http://reviewboard.vidsolbach.de/r/163/#comment114> {}s trunk/KDE/kdeplasma-addons/applets/fileWatcher/fileWatcher.cpp <http://reviewboard.vidsolbach.de/r/163/#comment115> spaces around '=' - Aaron On 2008-08-27 15:35:44, michal seben wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.vidsolbach.de/r/163/ > ----------------------------------------------------------- > > (Updated 2008-08-27 15:35:44) > > > Review request for Plasma. > > > Summary > ------- > > I made patch which allow automaticaly resize fileWatcher plasmoid, according > to the longest showed line, you could switch on/off auto resize in > fileWatcher config :) > > > Diffs > ----- > > trunk/KDE/kdeplasma-addons/applets/fileWatcher/fileWatcher.h > trunk/KDE/kdeplasma-addons/applets/fileWatcher/fileWatcher.cpp > trunk/KDE/kdeplasma-addons/applets/fileWatcher/fileWatcherConfig.h > trunk/KDE/kdeplasma-addons/applets/fileWatcher/fileWatcherConfig.cpp > trunk/KDE/kdeplasma-addons/applets/fileWatcher/fileWatcherConfig.ui > > Diff: http://reviewboard.vidsolbach.de/r/163/diff > > > Testing > ------- > > > Screenshots > ----------- > > Changed config > http://reviewboard.vidsolbach.de/r/163/s/48/ > > > Thanks, > > michal > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
