Use destinationDir to save viewproperties when not in global viewproperties setting
Needs ReviewPublic

Authored by meven on Jun 20 2019, 4:31 PM.

Details

Reviewers
ngraham
elvisangelaccio
Group Reviewers
Dolphin
Summary

Use the destinationDir to store viewproperties outside of user' folders.
Add a code path to migrate users settings, to be removed later.
Settings are saved in /home/$USER/.local/share/dolphin/view_properties

BUG: 322922
CC-BUG: 393960
FIXED-IN: 19.08

Diff Detail

Repository
R318 Dolphin
Branch
destinationDir.directory
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 13052
Build 13070: arc lint + arc unit
meven created this revision.Jun 20 2019, 4:31 PM
Restricted Application added a project: Dolphin. · View Herald TranscriptJun 20 2019, 4:31 PM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
meven requested review of this revision.Jun 20 2019, 4:31 PM
meven edited the summary of this revision. (Show Details)Jun 20 2019, 9:52 PM

This should make a lot of people happy. :) But what happens when a folder is moved?

src/views/viewproperties.cpp
84–99

prior to fix to 322922 -> prior to fixing 322922

85

version -> versions

meven updated this revision to Diff 60172.Jun 20 2019, 9:57 PM

Fixing spelling

meven updated this revision to Diff 60173.Jun 20 2019, 9:58 PM
meven marked 2 inline comments as done.

Spelling 2

ngraham added inline comments.Jun 20 2019, 10:44 PM
src/views/viewproperties.cpp
89

Don't use QDir::separator(); Qt automatically translates slashes as needed on Windows.

https://agateau.com/2015/qdir-separator-considered-harmful/

meven edited the summary of this revision. (Show Details)Jun 20 2019, 10:46 PM
meven updated this revision to Diff 60177.Jun 21 2019, 7:06 AM

Use QLatin1Char('/') instead QDir::separator()

meven edited the summary of this revision. (Show Details)Jun 21 2019, 1:40 PM
meven added a comment.Jun 21 2019, 1:50 PM

A limitation of the current patch is when a folder is moved, the settings are lost.
I can imagine two potential solution :

  • a "bazooka" solution : use inotify and consort to have the settings follow the files.
  • a simple solution : use inode id as path in destinationDir for local files. That way if a folder is moved its settings follow. Although it won't survive network moves.

Using inodes are probably the better approach. inotify won't work because there there are only a limited number of them and Baloo already consumes a ton for its purposes. Not surviving network moves isn't critical--or even important at all; I don't think most people expect settings like these to be transferred along with the files.

A third solution would be to store the view settings in an extended attribute on the folder itself. That wouldn't survive copying to or from filesystems that don't support xattrs though.

Sorry, I missed this one. I'd say let's postpone to 19.12...

meven added a comment.Jul 18 2019, 6:40 AM

Sorry, I missed this one. I'd say let's postpone to 19.12...

No worries

Also we might want to discuss it a bit more, folder moving :

A limitation of the current patch is when a folder is moved, the settings are lost.
I can imagine two potential solution :

  • a "bazooka" solution : use inotify and consort to have the settings follow the files.
  • a simple solution : use inode id as path in destinationDir for local files. That way if a folder is moved its settings follow. Although it won't survive network moves.

A third solution would be to store the view settings in an extended attribute on the folder itself. That wouldn't survive copying to or from filesystems that don't support xattrs though.

Sorry, I missed this one. I'd say let's postpone to 19.12...

No worries

Also we might want to discuss it a bit more, folder moving :

A limitation of the current patch is when a folder is moved, the settings are lost.
I can imagine two potential solution :

  • a "bazooka" solution : use inotify and consort to have the settings follow the files.
  • a simple solution : use inode id as path in destinationDir for local files. That way if a folder is moved its settings follow. Although it won't survive network moves.

A third solution would be to store the view settings in an extended attribute on the folder itself. That wouldn't survive copying to or from filesystems that don't support xattrs though.

Not sure what to do here, honestly.

  • bazooka solution: sounds dangerous. And what if inotify is not available?
  • inode id solution: doesn't look portable, what about Windows?
  • xattr solution: same, not portable
meven added a comment.Jan 14 2021, 6:33 AM

Sorry, I missed this one. I'd say let's postpone to 19.12...

No worries

Also we might want to discuss it a bit more, folder moving :

A limitation of the current patch is when a folder is moved, the settings are lost.
I can imagine two potential solution :

  • a "bazooka" solution : use inotify and consort to have the settings follow the files.
  • a simple solution : use inode id as path in destinationDir for local files. That way if a folder is moved its settings follow. Although it won't survive network moves.

A third solution would be to store the view settings in an extended attribute on the folder itself. That wouldn't survive copying to or from filesystems that don't support xattrs though.

Not sure what to do here, honestly.

  • bazooka solution: sounds dangerous. And what if inotify is not available?
  • inode id solution: doesn't look portable, what about Windows?
  • xattr solution: same, not portable

Well Windows has its own FS id https://docs.microsoft.com/fr-fr/windows/win32/api/fileapi/nf-fileapi-getfileinformationbyhandle?redirectedfrom=MSDN
We could manage to have a compatibility logic.