D15237: [ViewProperties] Check part of home first before doing file system stuff

[Kai Uwe Broulik]

broulik created this revision.
broulik added reviewers: Dolphin, elvisangelaccio, lbeltrame.
Herald added a project: Dolphin.
Herald added a subscriber: kfm-devel.
broulik requested review of this revision.

REVISION SUMMARY
There's no point in creating a `QFileInfo` instance and checking for file properties if we're not going to do anything with it when not inside home

TEST PLAN
- Accessing a slow or stale mount in `/run/user/foo/bar` no longer blocks here and is a nice simple optimization anyway imho

REPOSITORY
R318 Dolphin

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

AFFECTED FILES
src/views/viewproperties.cpp

To: broulik, #dolphin, elvisangelaccio, lbeltrame
Cc: kfm-devel, spoorun, navarromorales, firef, andrebarros, emmanuelp

[Luca Beltrame]

lbeltrame added a comment.


I don't understand well the is part of home stuff, do you mean checking for things out of /home?

REPOSITORY
R318 Dolphin

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

To: broulik, #dolphin, elvisangelaccio, lbeltrame
Cc: kfm-devel, spoorun, navarromorales, firef, andrebarros, emmanuelp

[Kai Uwe Broulik]

broulik added a comment.


From what I understand if the file is not inside `/home/user` then it will not even try writing into the `.directory` file of a folder but always save it in Dolphin's config folder

REPOSITORY
R318 Dolphin

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

To: broulik, #dolphin, elvisangelaccio, lbeltrame
Cc: kfm-devel, spoorun, navarromorales, firef, andrebarros, emmanuelp

[Kai Uwe Broulik]

broulik added a comment.


Ping

REPOSITORY
R318 Dolphin

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

To: broulik, #dolphin, elvisangelaccio, lbeltrame
Cc: kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp

[Luca Beltrame]

lbeltrame added a comment.


+1, but someone else must approve.

REPOSITORY
R318 Dolphin

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

To: broulik, #dolphin, elvisangelaccio, lbeltrame
Cc: kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp

[Elvis Angelaccio]

elvisangelaccio accepted this revision.
elvisangelaccio added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

viewproperties.cpp:71-73
+ // Check if the directory is writable and check if the ".directory" file exists and
+ // is read- and writable.
+ // Also check size of directory > 0 before accessing files inside it (stale mount)

I'd just remove these comments, because they are a bit confusing (and we already have a better comment at the top of the if-else chain).

REPOSITORY
R318 Dolphin

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

To: broulik, #dolphin, elvisangelaccio, lbeltrame
Cc: kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R318:7ec783e7493a: [ViewProperties] Check part of home first before doing file system stuff (authored by broulik).

CHANGED PRIOR TO COMMIT
https://phabricator.kde.org/D15237?vs=40904&id=41842#toc

REPOSITORY
R318 Dolphin

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D15237?vs=40904&id=41842

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

AFFECTED FILES
src/views/viewproperties.cpp

To: broulik, #dolphin, elvisangelaccio, lbeltrame
Cc: kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp