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

wilderkde at gmail.com wilderkde at gmail.com
Tue Feb 24 21:15:14 CET 2009


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

(Updated 2009-02-24 12:15:13.669339)


Review request for Plasma.


Changes
-------

in a previous commit (which actually applied a patch that I proposed, *doh* ) m_queryRunning was set to false even if the last query did not start regardless its previous state; this patch solves *this* problem:
if i type the /complete/ queryterm and hit enter the m_queryRunning is erroneously set to false regardless of its current state, so the old query it's free to run.
 
Still, I don't know if the original problem is solved, old results are still delivered to setQueryMatches so in principle if one manages to 
set m_delayedRun BEFORE the call, the call for old matches will still set m_queryRunning to false and run the item, regardless of the bug above. 
Is that situation possible? 

I'm reluctant to use ||= to avoid lazy OR things or other optimization quirks. [I kind of remember that (a || b) can be compiled differently than (b||a); am I wrong?]

I can commit the fix once I have your green light.

--J


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 (updated)
-----

  /branches/KDE/4.2/kdebase/workspace/krunner/interfaces/default/interface.cpp 931111 

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