dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  +1 for the included unittest.

INLINE COMMENTS

> kdeplatformfiledialoghelper.cpp:80
>   */
> -static QString kde2QtFilter(const QStringList &list, const QString &kde)
> +static QString kde2QtFilter(const QStringList &list, const QString &kde, 
> const QString &filterText)
>  {

(hehe I keep thinking this is about KDE-2 stuff... what a dinosaur I am)

> kdeplatformfiledialoghelper.cpp:90
> +                (QLatin1Char(')') == (*it)[pos + kde.length()] || 
> QLatin1Char(' ') == (*it)[pos + kde.length()]) &&
> +                (filterText.isEmpty() || (!filterText.isEmpty() && 
> it->startsWith(filterText)))) {
>              return *it;

the `!filterText.isEmpty() &&` part is redundant.
In this part of the condition, we know it's not empty, otherwise shortcut 
evaluation happened after isEmpty returns true.

  blue or (not blue and green)

is really the same as

  blue or green

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D21959

To: hoffmannrobert, #frameworks, apol, dfaure
Cc: michaelweghorn, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

Reply via email to