D14893: [recentdocuments:/] Filter out files that can't be browsed with a file manager

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: broulik, Dolphin, Frameworks.
ngraham requested review of this revision.

REVISION SUMMARY
This is a more appropriate version of D13797: Only add actual documents to Recent Documents <https://phabricator.kde.org/D13797>. Basically we filter out anything that can't be browsed with a file manager, such as web linkns, `appstream://` urls, etc.

This ensures that the contents of the `recentdocuments:/` ioslave is relevant to the user.

TEST PLAN
Opened a link from Spectacle in my web browser. Without the patch, it shows up in `recentdocuments:/`. With the patch, it does not.

REPOSITORY
R320 KIO Extras

BRANCH
master

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

AFFECTED FILES
recentdocuments/recentdocuments.cpp

To: ngraham, broulik, #dolphin, #frameworks

[Mark Gaiser]

markg added a comment.


It "looks" oke to me, but i don't know this code one bit.

INLINE COMMENTS

recentdocuments.cpp:82-83
+ if (urlInside.scheme() == "recentdocuments"
+ // Filter out things that can't be viewed in a file manager because they don't
+ // meet the user definition of a file for the purpose of "recently accessed files"
+ || !KProtocolManager::supportsListing(urlInside)

Comments inside a multi-line if statement! Unheard of!
Please move that to a more appropriate place.

REPOSITORY
R320 KIO Extras

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

To: ngraham, broulik, #dolphin, #frameworks
Cc: markg

[Nathaniel Graham]

ngraham marked an inline comment as done.

REPOSITORY
R320 KIO Extras

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

To: ngraham, broulik, #dolphin, #frameworks
Cc: markg

[Nathaniel Graham]

ngraham updated this revision to Diff 40054.
ngraham added a comment.


Move comment

REPOSITORY
R320 KIO Extras

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D14893?vs=39892&id=40054

BRANCH
arcpatch-D14893

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

AFFECTED FILES
recentdocuments/recentdocuments.cpp

To: ngraham, broulik, #dolphin, #frameworks
Cc: markg

[Nathaniel Graham]

ngraham added a comment.

Post by Mark Gaiser
It "looks" oke to me, but i don't know this code one bit.

Give it a try, it's pretty easy to test with the test plan.

REPOSITORY
R320 KIO Extras

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

To: ngraham, broulik, #dolphin, #frameworks
Cc: markg

[Elvis Angelaccio]

elvisangelaccio added a comment.


+1, lgtm

REPOSITORY
R320 KIO Extras

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

To: ngraham, broulik, #dolphin, #frameworks
Cc: elvisangelaccio, markg

[Nathaniel Graham]

This revision was not accepted when it landed; it landed in state "Needs Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R320:8606d4d97f3e: [recentdocuments:/] Filter out files that can&#039;t be browsed with a file manager (authored by ngraham).

REPOSITORY
R320 KIO Extras

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D14893?vs=40054&id=40074

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

AFFECTED FILES
recentdocuments/recentdocuments.cpp

To: ngraham, broulik, #dolphin, #frameworks
Cc: elvisangelaccio, markg

[Nathaniel Graham]

ngraham added a task: T8349: Improve Places panel usability and presentation.
Herald added projects: Dolphin, Frameworks.
Herald added subscribers: kfm-devel, kde-frameworks-devel.

REPOSITORY
R320 KIO Extras

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

To: ngraham, broulik, #dolphin, #frameworks
Cc: kde-frameworks-devel, kfm-devel, elvisangelaccio, markg, feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp