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

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to