D14043: [KFilePlacesModel] Support pretty baloosearch URLs

[Dominik Haumann]

dhaumann added inline comments.

INLINE COMMENTS

> kfileplacesmodel.cpp:137
>  
> -if (path.endsWith(QLatin1String("/documents"))) {
> -searchUrl = searchUrlForType(QStringLiteral("Document"));
> -} else if (path.endsWith(QLatin1String("/images"))) {
> -searchUrl = searchUrlForType(QStringLiteral("Image"));
> -} else if (path.endsWith(QLatin1String("/audio"))) {
> -searchUrl = searchUrlForType(QStringLiteral("Audio"));
> -} else if (path.endsWith(QLatin1String("/videos"))) {
> -searchUrl = searchUrlForType(QStringLiteral("Video"));
> -} else {
> -qWarning() << "Invalid search url:" << url;
> -searchUrl = url;
> +const QStringList validSearchPaths = {
> +QStringLiteral("/documents"),

const auto validSearchPaths = { ... }; Would use an initializer list. This 
would be likely faster, since it would avoid the QStringList.

REPOSITORY
  R241 KIO

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

To: broulik, #frameworks, bruns, dfaure, renatoo, franckarrecot, ngraham, hein
Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D14043: [KFilePlacesModel] Support pretty baloosearch URLs

[David Faure]

dfaure added a comment.


  This change broke KFilePlacesViewTest, please update that test as well.
  
  $ bin/kfileplacesviewtest
  
  - Start testing of KFilePlacesViewTest *
  
  Config: Using QtTest library 5.11.1, Qt 5.11.1 (x86_64-little_endian-lp64 
shared (dynamic) debug build; by GCC 5.3.1 20160301 [gcc-5-branch revision 
233849])
  PASS   : KFilePlacesViewTest::initTestCase()
  PASS   : KFilePlacesViewTest::testUrlChanged(Today)
  PASS   : KFilePlacesViewTest::testUrlChanged(Yesterday)
  FAIL!  : KFilePlacesViewTest::testUrlChanged(Documents) Compared values are 
not the same
  
Actual   (args.at(0).toUrl().toString()): "baloosearch:/documents"
Expected (expectedUrl)  : 
"baloosearch:?json=%7B%22dayFilter%22: 0, %22monthFilter%22: 0, 
%22yearFilter%22: 0, %22type%22: [ %22Document%22]%7D"
Loc: [/d/kde/src/5/frameworks/kio/autotests/kfileplacesviewtest.cpp(114)]
  
  FAIL!  : KFilePlacesViewTest::testUrlChanged(Images) Compared values are not 
the same
  
Actual   (args.at(0).toUrl().toString()): "baloosearch:/images"
Expected (expectedUrl)  : 
"baloosearch:?json=%7B%22dayFilter%22: 0, %22monthFilter%22: 0, 
%22yearFilter%22: 0, %22type%22: [ %22Image%22]%7D"
Loc: [/d/kde/src/5/frameworks/kio/autotests/kfileplacesviewtest.cpp(114)]
  
  FAIL!  : KFilePlacesViewTest::testUrlChanged(Audio Files) Compared values are 
not the same
  
Actual   (args.at(0).toUrl().toString()): "baloosearch:/audio"
Expected (expectedUrl)  : 
"baloosearch:?json=%7B%22dayFilter%22: 0, %22monthFilter%22: 0, 
%22yearFilter%22: 0, %22type%22: [ %22Audio%22]%7D"
Loc: [/d/kde/src/5/frameworks/kio/autotests/kfileplacesviewtest.cpp(114)]
  
  FAIL!  : KFilePlacesViewTest::testUrlChanged(Videos) Compared values are not 
the same
  
Actual   (args.at(0).toUrl().toString()): "baloosearch:/videos"
Expected (expectedUrl)  : 
"baloosearch:?json=%7B%22dayFilter%22: 0, %22monthFilter%22: 0, 
%22yearFilter%22: 0, %22type%22: [ %22Video%22]%7D"
Loc: [/d/kde/src/5/frameworks/kio/autotests/kfileplacesviewtest.cpp(114)]
  
  PASS   : KFilePlacesViewTest::cleanupTestCase()
  Totals: 4 passed, 4 failed, 0 skipped, 0 blacklisted, 891ms
  
  - Finished testing of KFilePlacesViewTest *

REPOSITORY
  R241 KIO

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

To: broulik, #frameworks, bruns, dfaure, renatoo, franckarrecot, ngraham, hein
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14043: [KFilePlacesModel] Support pretty baloosearch URLs

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:3523ba575b39: [KFilePlacesModel] Support pretty 
baloosearch URLs (authored by broulik).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14043?vs=37553=37858

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

AFFECTED FILES
  autotests/kfileplacesmodeltest.cpp
  src/filewidgets/kfileplacesmodel.cpp

To: broulik, #frameworks, bruns, dfaure, renatoo, franckarrecot, ngraham, hein
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14043: [KFilePlacesModel] Support pretty baloosearch URLs

[Eike Hein]

hein accepted this revision.

REPOSITORY
  R241 KIO

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

To: broulik, #frameworks, bruns, dfaure, renatoo, franckarrecot, ngraham, hein
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14043: [KFilePlacesModel] Support pretty baloosearch URLs

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Tested with a new and existing user. Entries in Kickoff, Dolphin, Gwenview, 
and the file dialogs all work perfectly, and the subtitles in Kickoff are 
pretty. Code looks sane.

REPOSITORY
  R241 KIO

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

To: broulik, #frameworks, bruns, dfaure, renatoo, franckarrecot, ngraham, hein
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14043: [KFilePlacesModel] Support pretty baloosearch URLs

[Kai Uwe Broulik]

broulik edited the test plan for this revision.
broulik added a dependency: D14042: Support special URLs for finding files of a 
certain type.

REPOSITORY
  R241 KIO

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

To: broulik, #frameworks, bruns, dfaure, renatoo, franckarrecot, ngraham, hein
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14043: [KFilePlacesModel] Support pretty baloosearch URLs

[Kai Uwe Broulik]

broulik created this revision.
broulik added reviewers: Frameworks, bruns, dfaure, renatoo, franckarrecot, 
ngraham, hein.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
broulik requested review of this revision.

REVISION SUMMARY
  D14042  moves the logic into the KIO slave

TEST PLAN
  Adjusted test, passes
  All "Search For" default entries in Dolphin still work
  Kickoff shows pretty search URLs now

REPOSITORY
  R241 KIO

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

AFFECTED FILES
  autotests/kfileplacesmodeltest.cpp
  src/filewidgets/kfileplacesmodel.cpp

To: broulik, #frameworks, bruns, dfaure, renatoo, franckarrecot, ngraham, hein
Cc: kde-frameworks-devel, michaelh, ngraham, bruns