| bruns | |
| cfeck |
| Baloo | |
| Dolphin | |
| Frameworks |
balooctl add includeFolders/excludeFolders was checking for parent directories before allowing the user to add a subdirectory. This is unnecessary since Baloo itself supports the logic of (for example) excluding ~/foo but including ~/foo/bar. Removing the check solves the problem and fixes 396535.
BUG: 396535
FIXED-IN: 5.51
mkdir ~/test1 ~/test2 ~/test1-test ~/test1-test/foo balooctl config add excludeFolders ~/test1 balooctl config add excludeFolders ~/test2 balooctl config add excludeFolders ~/test1-test balooctl config add includeFolders ~/test1-test/foo
If it worked, you don't get any errors.
| No Linters Available |
| No Unit Test Coverage |
| Buildable 3128 | |
| Build 3146: arc lint + arc unit |
In Qt 5.9 (if i remember correct) was introduced to not have trailing '/' so, QDir::separator should not be used, so better get folder as copy
for (QString folder : folders) {
if (!folder.endsWith(QLatin1Char('/')) {
folder += QLaitin1Char('/');
}
if (path.startsWith(folder)) {Actually it looks like we shouldn't even use it anyway for other reasons per the documentation: https://doc.qt.io/qt-5/qdir.html#separator
They recommend always using / directly like you indicate, so I'll do that.
I think the whole startsWith is flawed - it should be possible to have a e.g. "/home/user/foo/bar" include when "/home/user/foo" has been excluded.
Hmm, I'm not sure how much that matches the user expectation. If a folder is explicitly marked as excluded, I think it's most commonly understood that all its sub-folders would also be excluded.
I could see the case for allowing this behavior to be explicitly overridden by an advanced user who marks ~/foo/ as excluded and then later marks ~foo/bar/ as included, but that would be material for another patch I think.
If I read the unit tests correctly (https://phabricator.kde.org/source/baloo/browse/master/autotests/unit/file/fileindexerconfigtest.cpp), this is already supported. You just can not do it via balooctl.
Technically, this check isn't actually needed, though it does prevent the user from entering a path more than once. At first glance this looks like it should work:
if (folder.startsWith(path))
This doesn't prevent the user from also re-specifying valid paths.
Exact matches are already rejected in the block above.
folder.startsWith(path) is also pointless, a user should be allowed to e.g. exclude <home>/foo/ if <home>/foo/bar/baz is excluded and <home>/foo/bar is included.
What was wrong with https://phabricator.kde.org/D15583?id=42018 ?
I could see the case for allowing this behavior to be explicitly overridden by an advanced user who marks ~/foo/ as excluded and then later marks ~foo/bar/ as included, but that would be material for another patch I think.
This is actually already how this works, the excludes and includes are both resolved into either an index path or do not index path outcome.
https://phabricator.kde.org/D15583?id=42018 didn't let you add ~/foo/bar to the list of includes or excludes if ~/foo was already present.