bruns added a comment.
In D16643#405468 <https://phabricator.kde.org/D16643#405468>, @fvogt wrote: > > The second change fvogt mentioned is IMHO handled quite strangely here - !m_enabled || m_temporaryInhibition is the inverse of !m_enabled || m_temporaryInhibition, so doing a event->ignore() === event->setAccepted(false) prior to the first return statement would cover the ignored case, and allows the event to bubble up. > > Sure, but AFAICT that's just a matter of taste whether to `ignore(); if(!something) return; do stuff; accept();` or `accept(something); if(!something) return; do stuff;`. Its about the duplicate evaluation of ` !m_enabled || m_temporaryInhibition`. Expecially as it is not easy to spot both conditions are exactly the same. compare event->setAccepted(m_enabled && !m_temporaryInhibition); if (!m_enabled || m_temporaryInhibition) { return; } with if (!m_enabled || m_temporaryInhibition) { event->ignore(); return; } event->accept(); REPOSITORY R296 KDeclarative REVISION DETAIL https://phabricator.kde.org/D16643 To: trmdi, mart, broulik, #plasma, hein, bruns Cc: fvogt, aacid, bruns, dkorth, ngraham, kde-frameworks-devel, michaelh
