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

wilderkde at gmail.com wilderkde at gmail.com
Tue Feb 24 14:37:02 CET 2009



> On 2009-02-23 22:48:21, Aaron Seigo wrote:
> > 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.

Oh, now I understand what should happen. The problem is that, somehow, it doesn't work. Matches relative to old queries still pollute the results for new queries, and this is substantiated by kDebug evidence. 
I'll investigate about what exactly happens with the detaching. 
Thanks for the tips.

--J


- wilder


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/172/#review255
-----------------------------------------------------------


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



More information about the Plasma-devel mailing list