D21959: Fix selectedNameFilter() multiple matches

2019-06-27 Thread David Faure
dfaure closed this revision.

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


D21959: Fix selectedNameFilter() multiple matches

2019-06-24 Thread Robert Hoffmann
hoffmannrobert added a comment.


  Can you please push it for me, I don't have commit access. Thanks.

REPOSITORY
  R135 Integration for Qt applications in Plasma

BRANCH
  fix_selectedNameFilter

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


D21959: Fix selectedNameFilter() multiple matches

2019-06-22 Thread David Faure
dfaure accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R135 Integration for Qt applications in Plasma

BRANCH
  fix_selectedNameFilter

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


D21959: Fix selectedNameFilter() multiple matches

2019-06-22 Thread Robert Hoffmann
hoffmannrobert marked an inline comment as done.

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


D21959: Fix selectedNameFilter() multiple matches

2019-06-22 Thread Robert Hoffmann
hoffmannrobert updated this revision to Diff 60286.
hoffmannrobert added a comment.


  - Remove redundant condition

REPOSITORY
  R135 Integration for Qt applications in Plasma

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21959?vs=60217=60286

BRANCH
  fix_selectedNameFilter

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

AFFECTED FILES
  autotests/kfiledialog_unittest.cpp
  src/platformtheme/kdeplatformfiledialogbase_p.h
  src/platformtheme/kdeplatformfiledialoghelper.cpp
  src/platformtheme/kdeplatformfiledialoghelper.h
  src/platformtheme/kdirselectdialog.cpp
  src/platformtheme/kdirselectdialog_p.h

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


D21959: Fix selectedNameFilter() multiple matches

2019-06-22 Thread Nathaniel Graham
ngraham edited the test plan for this revision.

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


D21959: Fix selectedNameFilter() multiple matches

2019-06-22 Thread David Faure
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 , const QString )
> +static QString kde2QtFilter(const QStringList , const QString , 
> const QString )
>  {

(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


D21959: Fix selectedNameFilter() multiple matches

2019-06-21 Thread Nathaniel Graham
ngraham added reviewers: Frameworks, apol, dfaure.

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

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