----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/172/#review255 -----------------------------------------------------------
i don't think this patch is correct. the idea is that when the query changes, the context changes and we detach it. that means that the version referenced in all the threads is no longer accessible from the thread RunnerManager is in. this results in two things: a) the context in RunnerManager has no matches in it, and the threads can call addMatch all they want, it happens in a context that we don't care about anymore (because RunnerManager no longer references it). this mechanism was perhaps broken at some point, but the whole idea is to avoid copying data around and storing it in N places as in your patches here. /branches/KDE/4.2/kdebase/workspace/krunner/interfaces/default/interface.cpp <http://reviewboard.kde.org/r/172/#comment145> you don't need to initialize qstrings. /branches/KDE/4.2/kdebase/workspace/krunner/interfaces/default/interface.cpp <http://reviewboard.kde.org/r/172/#comment146> spaces around the = - Aaron On 2009-02-23 22:04:51, wilder wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/172/ > ----------------------------------------------------------- > > (Updated 2009-02-23 22:04:51) > > > Review request for Plasma. > > > Summary > ------- > > This is an attempt to solve https://bugs.kde.org/show_bug.cgi?id=169283 > > The problem with the current implementation is that the mechanism for > blocking a run of a result of a previous query does not work, as explained in > my comment (the last one) in the br. > > The proposed way to solve this issue is to store the query term in > Plasma::QueryMatch and then check that it matches the last query before > actually running the item. A patch for kdelibs will follow. > > In this way, I believe that m_queryRunning becomes irrelevant, but I still > have to think about it for some more. > > I tried to follow all guidelines for binary compatibility in kdelibs, but > please double check that. Also I think I can add the declaration of a new > method wherever I want, but just to be sure I put them at the end. If you > confirm that their position won't matter I'll put them in a more consistent > manner. > > --J > > > This addresses bugs 169283, et and similia. > https://bugs.kde.org/show_bug.cgi?id=169283 > https://bugs.kde.org/show_bug.cgi?id=et > https://bugs.kde.org/show_bug.cgi?id=similia > > > Diffs > ----- > > /branches/KDE/4.2/kdebase/workspace/krunner/interfaces/default/interface.h > 930601 > > /branches/KDE/4.2/kdebase/workspace/krunner/interfaces/default/interface.cpp > 930601 > /branches/KDE/4.2/kdebase/workspace/krunner/interfaces/default/resultitem.h > 930601 > > /branches/KDE/4.2/kdebase/workspace/krunner/interfaces/default/resultitem.cpp > 930601 > > Diff: http://reviewboard.kde.org/r/172/diff > > > Testing > ------- > > The patch appears to solve the bug. > > > Thanks, > > wilder > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
