Review Request: Prevent krunner to run the wrong command when "fast" typing

[Aaron Seigo]

-----------------------------------------------------------
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
> 
>