D10803: handle more tags in taglibextractor

2018-04-07 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > astippich wrote in taglibextractortest.cpp:82 > Hmm, I disagree since they are simple and they don't leave room for ambiguity Maybe just add a Prefix or Suffix for all QStrings? Same applies to any Int values, you can never tell if the right propert

D12037: Immediately apply termInConstruction when term is complete

2018-04-07 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, michaelh. Restricted Application added projects: Frameworks, Baloo. Restricted Application added a subscriber: Frameworks. bruns requested review of this revision. REVISION SUMMARY Term::Or and Term::And delimit the term in construction,

D12037: Immediately apply termInConstruction when term is complete

2018-04-07 Thread Stefan Brüns
bruns added dependencies: D11888: Handle adjacent special characters correctly, D12007: Add test case for parsing of double opening '(('. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12037 To: bruns, #baloo, michaelh Cc: #frameworks, ashaposhnikov, michaelh, astippich,

D11888: Handle adjacent special characters correctly

2018-04-07 Thread Stefan Brüns
bruns added a dependent revision: D12037: Immediately apply termInConstruction when term is complete. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D11888 To: bruns, #baloo, michaelh Cc: #frameworks, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexeymin

D12007: Add test case for parsing of double opening '(('

2018-04-07 Thread Stefan Brüns
bruns added a dependent revision: D12037: Immediately apply termInConstruction when term is complete. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12007 To: bruns, #baloo, michaelh Cc: #frameworks, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexeymin

D12007: Add test case for parsing of double opening '(('

2018-04-07 Thread Stefan Brüns
bruns updated this revision to Diff 31634. REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12007?vs=31547&id=31634 BRANCH b#392620_unittest REVISION DETAIL https://phabricator.kde.org/D12007 AFFECTED FILES autotests/unit/lib/advancedqueryparsertest.cpp T

D12007: Add test case for parsing of double opening '(('

2018-04-07 Thread Stefan Brüns
bruns marked 2 inline comments as done. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12007 To: bruns, #baloo, michaelh Cc: #frameworks, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexeymin

D11888: Handle adjacent special characters correctly

2018-04-07 Thread Stefan Brüns
bruns updated this revision to Diff 31636. bruns added a task: T3: Add string "Your system default" to translation. bruns added a comment. rebase REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11888?vs=31550&id=31636 BRANCH t3 REVISION DETAIL https://ph

D12037: Immediately apply termInConstruction when term is complete

2018-04-07 Thread Stefan Brüns
bruns updated this revision to Diff 31638. bruns added a comment. rebase REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12037?vs=31632&id=31638 BRANCH b392620_fix_term_stack REVISION DETAIL https://phabricator.kde.org/D12037 AFFECTED FILES autotests/

D12037: Immediately apply termInConstruction when term is complete

2018-04-07 Thread Stefan Brüns
bruns removed a task: T2: sudoku activity string must be changed: ". REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12037 To: bruns, #baloo, michaelh Cc: #frameworks, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexeymin

D11888: Handle adjacent special characters correctly

2018-04-07 Thread Stefan Brüns
bruns removed a task: T3: Add string "Your system default" to translation. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D11888 To: bruns, #baloo, michaelh Cc: #frameworks, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexeymin

D12047: Avoid crash when reading corrupt data from document terms db

2018-04-08 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, michaelh. Restricted Application added projects: Frameworks, Baloo. Restricted Application added a subscriber: Frameworks. bruns requested review of this revision. REVISION SUMMARY The terms db contains terms, where each terms is stored

D12044: baloodb: Improve interface

2018-04-08 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > databasesanitizer.cpp:249 > if (!(accessFilter & IgnoreAvailable)) { > -out << QStringLiteral("%1").arg(info.accessible ? "+" : "!") << > sep; > +color = (useColors && !info.accessible) ? colors.first : > colors.sec

D12045: Clean up existing documentation

2018-04-08 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > README.md:5 > > -Baloo provides file searching and indexing. It does so by maintaining an > index of the contents > -of your files. > +Baloo is the file indexing and file search framework for KDE Plasma. It > focuses on speed and a very small mem

D12047: Avoid crash when reading corrupt data from document terms db

2018-04-08 Thread Stefan Brüns
bruns added a reviewer: ngraham. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12047 To: bruns, #baloo, michaelh, ngraham Cc: #frameworks, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexeymin

D12007: Add test case for parsing of double opening '(('

2018-04-08 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:4e69ffeab75f: Add test case for parsing of double opening '((' (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12007?vs=31634&id=31679 REVISION D

D11888: Handle adjacent special characters correctly

2018-04-08 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:116e55a2076e: Handle adjacent special characters correctly (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11888?vs=31636&id=31681 REVISION DETAI

D12037: Immediately apply termInConstruction when term is complete

2018-04-08 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:c7c5a0bffbea: Immediately apply termInConstruction when term is complete (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12037?vs=31638&id=31682

D12005: Fix merging of terms in the AdvancedQueryParser

2018-04-08 Thread Stefan Brüns
bruns updated this revision to Diff 31685. REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12005?vs=31585&id=31685 BRANCH baloo_term_cleanup_2 REVISION DETAIL https://phabricator.kde.org/D12005 AFFECTED FILES autotests/unit/lib/advancedqueryparsertest.cpp

D12045: Clean up existing documentation

2018-04-08 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > README.md:27 > +* Baloo follows the > [KDELibs](https://techbase.kde.org/Policies/Kdelibs_Coding_Style) coding > style. > +* [![Build > Status](https://build.kde.org/job/Frameworks%20baloo%20kf5-qt5%20SUSEQt5.9/lastBuild/)](https://build.kde.org/j

D12051: [UDisks] Correct handling of removable file systems

2018-04-08 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: ngraham, broulik. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. bruns requested review of this revision. REVISION SUMMARY Filesystems which have no fstab entry have an empty filepath

D12051: [UDisks] Correct handling of removable file systems

2018-04-08 Thread Stefan Brüns
bruns added a dependency: D12050: Make automounting work even if StorageAccess is ignored. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D12051 To: bruns, ngraham, broulik Cc: #frameworks, michaelh, ngraham, bruns

D12051: [UDisks] Correct handling of removable file systems

2018-04-08 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R245:122a6cd8989a: [UDisks] Correct handling of removable file systems (authored by bruns). REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12051?vs=31696&id=31712 REVISIO

D11452: sanitizer: Improve device listing

2018-04-09 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > databasesanitizer.cpp:159 > +// Create a hash to sum-up indexed items > QMultiHash usedDevices; > +for (const auto& info : infos) { I can not come up with a reason to use a MultiHash here in the first place - all that is ne

D11745: databasesanitizer: Use flags for filtering

2018-04-09 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > databasesanitizer.cpp:100 > QTextStream err(stderr); > for (quint64 id: keys) { > printProgress(err, i, max, 100); As you are not iterating over sorted keys, just directly iterate on the map. > databasesanitizer.cpp:1

D11828: Simplify orPostingIterator and make it faster

2018-04-09 Thread Stefan Brüns
bruns added a comment. In D11828#242372 , @michaelh wrote: > While reading in IDE realized `(*it)` resolves to `PostingIterator**` I got a little dizzy at first, then started to play with this code and came up with this. > In Constructor: >

D11828: Simplify orPostingIterator and make it faster

2018-04-09 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > michaelh wrote in orpostingiterator.cpp:71 > Newbie question: Does std::unique_ptr make sense here? When writing from scratch today, yes. But this would not disallow nullptr (`std::make_unique(nullptr)` is fully valid) REPOSITORY R293 Baloo REV

D11828: Simplify orPostingIterator and make it faster

2018-04-09 Thread Stefan Brüns
bruns added a comment. In D11828#242402 , @michaelh wrote: > I'm not very familiar with the concept of iterators (yet). To me it looks like `auto i = new OrPostingIterator(iters); i->docId();` will return 0 and `i->next()` returns a valid docId.

D11452: sanitizer: Improve device listing

2018-04-09 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > michaelh wrote in fsutils.cpp:111 > I agree completely and followed your suggestion. Currently I'm stuck, though. > Using Solid to obtain the accessibilty info of volumes and network shares, > it seems only `Solid::Block` provides the `major` and

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2018-04-09 Thread Stefan Brüns
bruns added a comment. Restricted Application added a project: Baloo. The whole function as currently used is pointless. Currently, it sets the NO_COW flag on BTRFS, but this is insufficient: - XFS - https://lwn.net/Articles/747633/ - F2FS - Hammer2 ? At least on Linux, the c

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2018-04-09 Thread Stefan Brüns
bruns added a reviewer: bruns. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D5784 To: tcberner, #freebsd, kfunk, poboiko, bruns Cc: bruns, adridg, kfunk, #frameworks, ashaposhnikov, michaelh, astippich, spoorun, ngraham, alexeymin

D12045: Clean up existing documentation

2018-04-09 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > README.md:31 > +Baloo is developed and tested exclusively for Linux. While it may run on > other > +unix based systems. It is not recommended, and certainly not tested. > + While it may run on other unix based systems ", it is hardly tested. It is

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2018-04-09 Thread Stefan Brüns
bruns added a comment. In D5784#243432 , @adridg wrote: > The question is mostly: does the existing (complicated) code do anything that QStorageInfo doesn't? Because switching to QStorageInfo gives us the functionality on FreeBSD for free (even i

D11452: sanitizer: Improve device listing

2018-04-09 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > databasesanitizer.cpp:163 > +DeviceInfo& deviceInfo = usedDevices[info.deviceId]; > +if (deviceInfo.id == 0) { > +deviceInfo.id = info.deviceId; instead of checking and filling it in here, just iterate over us

D11745: databasesanitizer: Use flags for filtering

2018-04-10 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > databasesanitizer.cpp:116 > if ((!includeIds.isEmpty() && > !includeIds.contains(info.deviceId)) > || (!excludeIds.isEmpty() && > excludeIds.contains(info.deviceId)) > || (urlFilter && !urlFilter->matc

D12034: Use the more appropriate "two sliders" icon for "configure"

2018-04-10 Thread Stefan Brüns
bruns added a comment. +1 for making it more consistent ... For the design - how about this one (Shortcuts - old, Toolbars - new)? F5803306: Screenshot_20180410_224347.png REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.o

D12034: Use the more appropriate "two sliders" icon for "configure"

2018-04-10 Thread Stefan Brüns
bruns added a comment. Thats a new one REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D12034 To: ngraham, #vdg, #plasma, #breeze Cc: bruns, abetts, sharvey, #frameworks, michaelh, ngraham

D12123: [UDisks] Optimize several property checks

2018-04-11 Thread Stefan Brüns
bruns created this revision. bruns added a reviewer: Frameworks. Restricted Application added a project: Frameworks. bruns requested review of this revision. REVISION SUMMARY Use QStringLiteral for hasInterface argument Retrieve MountPoints propery just once when checking mount state. TEST

D12124: Avoid creating duplicate property entries in the cache

2018-04-11 Thread Stefan Brüns
bruns created this revision. bruns added a reviewer: Frameworks. Restricted Application added a project: Frameworks. bruns requested review of this revision. REVISION SUMMARY Properties are associated with a specific interface, although the Solid UDisks2 backend merges properties from all inte

D12124: Avoid creating duplicate property entries in the cache

2018-04-11 Thread Stefan Brüns
bruns updated this revision to Diff 31929. bruns added a comment. update, one line was missing REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12124?vs=31928&id=31929 BRANCH unique_properties REVISION DETAIL https://phabricator.kde.org/D12124 AFFECTED F

D12125: Avoid inserting an invalid "Size" property from the Filesystem interface

2018-04-11 Thread Stefan Brüns
bruns created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. bruns requested review of this revision. REVISION SUMMARY UDisks2 added a "Size" property for the Filesystem interface, which may be zero, denoting an unknown

D12126: Invalidate property cache when an interface is removed

2018-04-11 Thread Stefan Brüns
bruns created this revision. bruns added a reviewer: Frameworks. Restricted Application added a project: Frameworks. bruns requested review of this revision. REVISION SUMMARY As we do not know which property belongs to which interface we have to drop the whole cache whenever one or multiples i

D12127: Do not query properties when no interfaces are left

2018-04-11 Thread Stefan Brüns
bruns created this revision. bruns added a reviewer: Frameworks. Restricted Application added a project: Frameworks. bruns requested review of this revision. REVISION SUMMARY checkCache is indirectly invoked from DeviceManager::updatebackend(), e.g. after an InterfaceRemoved call. Avoid doing

D12028: taglibextractortest: Add test for files with empty metadata

2018-04-11 Thread Stefan Brüns
bruns added a comment. In D12028#244257 , @michaelh wrote: > In D12028#244243 , @mgallien wrote: > > > I need more time. I will try to look at it today. By the way, the stack concept seems very usef

D12028: taglibextractortest: Add test for files with empty metadata

2018-04-11 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > michaelh wrote in taglibextractortest.cpp:231 > This converts the enums to strings. That way it is easier to spot which > properties are responsible for the failure. What he likely meant: `TagLibExtractor plugin{this};` instead of `QScopedPointer p

D12125: Avoid inserting an invalid "Size" property from the Filesystem interface

2018-04-11 Thread Stefan Brüns
bruns added a dependency: D12124: Avoid creating duplicate property entries in the cache. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D12125 To: bruns Cc: #frameworks, michaelh, ngraham, bruns

D12124: Avoid creating duplicate property entries in the cache

2018-04-11 Thread Stefan Brüns
bruns added a dependent revision: D12125: Avoid inserting an invalid "Size" property from the Filesystem interface. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D12124 To: bruns, #frameworks Cc: michaelh, ngraham, bruns

D12045: Clean up existing documentation

2018-04-11 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > michaelh wrote in README.md:36 > - In CI Baloo is fully built and tested for Linux and FreeBSD. > - In CI an untested stub for Windows is build. I think that is to fulfill the > dependencies. > - macOS is not mentioned anywhere (and I don't know any

D12135: Allow disabling of CoW to fail when not supported by filesystem

2018-04-11 Thread Stefan Brüns
bruns created this revision. bruns added a reviewer: Baloo. Restricted Application added projects: Frameworks, Baloo. Restricted Application added a subscriber: Frameworks. bruns requested review of this revision. REVISION SUMMARY It is fine to neither support reading of attrs nor setting the NO

D12136: Avoid hardcoding of filesystems supporting CoW

2018-04-11 Thread Stefan Brüns
bruns created this revision. bruns added a reviewer: Baloo. Restricted Application added projects: Frameworks, Baloo. Restricted Application added a subscriber: Frameworks. bruns requested review of this revision. REVISION SUMMARY The correct way to check for CoW support and conditionally disabl

D12136: Avoid hardcoding of filesystems supporting CoW

2018-04-11 Thread Stefan Brüns
bruns added a dependency: D12135: Allow disabling of CoW to fail when not supported by filesystem. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12136 To: bruns, #baloo Cc: #freebsd, #frameworks, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexeymin

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2018-04-11 Thread Stefan Brüns
bruns added a comment. See D12136 REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D5784 To: tcberner, #freebsd, poboiko, bruns Cc: bruns, adridg, kfunk, #frameworks, ashaposhnikov, michaelh, astippich, spoorun, ngraham, alexeymin

D12135: Allow disabling of CoW to fail when not supported by filesystem

2018-04-11 Thread Stefan Brüns
bruns added a dependent revision: D12136: Avoid hardcoding of filesystems supporting CoW. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12135 To: bruns, #baloo Cc: #freebsd, #frameworks, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexeymin

D12135: Allow disabling of CoW to fail when not supported by filesystem

2018-04-11 Thread Stefan Brüns
bruns updated this revision to Diff 31946. bruns edited the test plan for this revision. bruns added a comment. update function documentation REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12135?vs=31943&id=31946 BRANCH accept_eopnotsupp_on_chattr REVISIO

D12136: Avoid hardcoding of filesystems supporting CoW

2018-04-11 Thread Stefan Brüns
bruns added a dependent revision: D12138: Remove FSUtils::getDirectoryFileSystem. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12136 To: bruns, #baloo Cc: #freebsd, #frameworks, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexeymin

D12138: Remove FSUtils::getDirectoryFileSystem

2018-04-11 Thread Stefan Brüns
bruns added a dependency: D12136: Avoid hardcoding of filesystems supporting CoW. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12138 To: bruns, #baloo, #freebsd Cc: #frameworks, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexeymin

D12138: Remove FSUtils::getDirectoryFileSystem

2018-04-11 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, FreeBSD. Restricted Application added projects: Frameworks, Baloo. Restricted Application added a subscriber: Frameworks. bruns requested review of this revision. REVISION SUMMARY It was only used to determine if the filesystem is BTRFS

D12130: Use the more user-friendly string "File format" in the open/save dialogs

2018-04-11 Thread Stefan Brüns
bruns accepted this revision. bruns added a comment. This revision is now accepted and ready to land. +1 REPOSITORY R241 KIO BRANCH file-format-label (branched from master) REVISION DETAIL https://phabricator.kde.org/D12130 To: ngraham, #frameworks, #vdg, bruns Cc: bruns, michaelh, ng

D12145: autotests: Do not use QScopedPointer for plugins

2018-04-12 Thread Stefan Brüns
bruns added a comment. The first question is - why should we use pointers at all (raw, std::unique_ptr, QScopedPointer, ...) for objects **with local only scope**. The typical answer is Inheritance, see e.g. http://doc.qt.io/qt-5/qscopedpointer.html#details A `Base` pointer can be used

D12135: Allow disabling of CoW to fail when not supported by filesystem

2018-04-12 Thread Stefan Brüns
bruns added a reviewer: Frameworks. REPOSITORY R293 Baloo BRANCH accept_eopnotsupp_on_chattr REVISION DETAIL https://phabricator.kde.org/D12135 To: bruns, #baloo, adridg, #frameworks Cc: adridg, #freebsd, #frameworks, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexeymin

D12135: Allow disabling of CoW to fail when not supported by filesystem

2018-04-12 Thread Stefan Brüns
bruns added a comment. May I have another accept/review (for the stack) from #Frameworks or #Baloo ? REPOSITORY R293 Baloo BRANCH accept_eopnotsupp_on_chattr REVISION DETAIL https://phabricator.kde

D12028: taglibextractortest: Add test for files with empty metadata

2018-04-12 Thread Stefan Brüns
bruns added a comment. Untested, but looks good to me otherwise. INLINE COMMENTS > taglibextractortest.cpp:42 > > +const QStringList TagLibExtractorTest::propertyEnumNames(const > QList& keys) const > +{ Nitpick - needs some indentation REPOSITORY R286 KFileMetaData REVISION DETAIL

D11452: sanitizer: Improve device listing

2018-04-12 Thread Stefan Brüns
bruns added a subscriber: ngraham. bruns added a comment. Looks good to me so far. If there are any issues, we can fix it up later IMHO. @ngraham as you reviewed other parts of the stack, can you do this one as well and accept? INLINE COMMENTS > databasesanitizer.cpp:257 > +<<

D12124: Avoid creating duplicate property entries in the cache

2018-04-12 Thread Stefan Brüns
bruns added a comment. In D12124#244924 , @broulik wrote: > Wouldn't make a big difference, would it? > > > If there are multiple items for key in the map, the value of the most recently inserted one is returned. > > It's not like we actu

D12125: Avoid inserting an invalid "Size" property from the Filesystem interface

2018-04-12 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > broulik wrote in udisksdevicebackend.cpp:147 > So when size is unknown it will always query it as you will never have a Size > in the `m_propertyCache`? It uses the one from the Partition or Block Interface, see last paragraph from the Summary. It

D12125: Avoid inserting an invalid "Size" property from the Filesystem interface

2018-04-12 Thread Stefan Brüns
bruns added a reviewer: Frameworks. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D12125 To: bruns, #frameworks Cc: broulik, #frameworks, michaelh, ngraham, bruns

D11452: sanitizer: Improve device listing

2018-04-12 Thread Stefan Brüns
bruns accepted this revision. bruns added a comment. This revision is now accepted and ready to land. Yes, thats exactly what I meant - device ids are not stable ... But anyway, thats unrelated to this revision. REPOSITORY R293 Baloo BRANCH sanitize-devices (branched from master) REVIS

D12028: taglibextractortest: Add test for files with empty metadata

2018-04-12 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > taglibextractortest.cpp:300 > +} > +QCOMPARE(resultKeys, expectedKeys); > +} Unfortunately QCOMPARE does not do a deep compare if sizes mismatch. To get a better output in case the test fails, you can do something like: auto excessKeys()

D12135: Allow disabling of CoW to fail when not supported by filesystem

2018-04-12 Thread Stefan Brüns
bruns requested review of this revision. bruns added a comment. Make it obvious this should have some more review ... REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12135 To: bruns, #baloo, adridg, #frameworks Cc: adridg, #freebsd, #frameworks, ashaposhnikov, michaelh

D12127: Do not query properties when no interfaces are left

2018-04-12 Thread Stefan Brüns
bruns added a comment. In D12127#244859 , @broulik wrote: > Lgtm. > But do we remove the offending properties when the interface is removed? Not sure, is probably unrelated bug Yes, D12126 REPOSITO

D11745: databasesanitizer: Use flags for filtering

2018-04-12 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > databasesanitizer.cpp:32 > #include > +#include > double QDebug REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D11745 To: michaelh, #baloo, #frameworks Cc: bruns, ngraham, smithjd, ashaposhnikov, michaelh, astippich, s

D11745: databasesanitizer: Use flags for filtering

2018-04-12 Thread Stefan Brüns
bruns accepted this revision. This revision is now accepted and ready to land. REPOSITORY R293 Baloo BRANCH sanitize-enums (branched from master) REVISION DETAIL https://phabricator.kde.org/D11745 To: michaelh, #baloo, #frameworks, bruns Cc: bruns, ngraham, smithjd, ashaposhnikov, michael

D12130: Use the more user-friendly string "File format" in the save dialogs

2018-04-12 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > ngraham wrote in kfilewidget.cpp:1338 > I've always said "save as." > > "Save that picture as a JPEG real fast, would ya?" "Use it to choose which format to use for saving"? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D121

D12130: Use the more user-friendly string "File format" in the save dialogs

2018-04-12 Thread Stefan Brüns
bruns added a comment. In D12130#245373 , @ngraham wrote: > Change the text only when saving (it really is a filter bar in open mode) Why not use "File formats" (plural) for the Open dialog? REPOSITORY R241 KIO REVISION DETAIL https:

D12123: [UDisks] Optimize several property checks

2018-04-12 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R245:3c0f767f0337: [UDisks] Optimize several property checks (authored by bruns). REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12123?vs=31927&id=32025 REVISION DETAIL

D12124: Avoid creating duplicate property entries in the cache

2018-04-12 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R245:15047128e56b: Avoid creating duplicate property entries in the cache (authored by bruns). REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12124?vs=31929&id=32026 REVI

D12126: Invalidate property cache when an interface is removed

2018-04-12 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R245:796dea6c0440: Invalidate property cache when an interface is removed (authored by bruns). REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12126?vs=31931&id=32027 REVI

D12028: taglibextractortest: Add test for files with empty metadata

2018-04-12 Thread Stefan Brüns
bruns accepted this revision. bruns added a comment. This revision is now accepted and ready to land. Ok, lets see Jenkins opinion ... REPOSITORY R286 KFileMetaData BRANCH nometa REVISION DETAIL https://phabricator.kde.org/D12028 To: michaelh, mgallien, #baloo, #frameworks, bruns Cc:

D11452: sanitizer: Improve device listing

2018-04-12 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > databasesanitizer.cpp:135 > +QMap result; > +const auto devices = FSUtils::attachedDevices(); > +for (const auto& dev : devices) { Completely overlooked this one - should have used `QStorageInfo::mountedVolumes()

D12165: Use QStorageInfo instead of a homegrown implementation

2018-04-12 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, michaelh. Restricted Application added projects: Frameworks, Baloo. Restricted Application added a subscriber: Frameworks. bruns requested review of this revision. REVISION SUMMARY The homegrown implementation only supports Linux TEST P

D12165: Use QStorageInfo instead of a homegrown implementation

2018-04-12 Thread Stefan Brüns
bruns added a comment. Should probably land before D11745 REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12165 To: bruns, #baloo, michaelh Cc: #frameworks, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexeymin

D11745: databasesanitizer: Use flags for filtering

2018-04-12 Thread Stefan Brüns
bruns requested changes to this revision. bruns added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > databasesanitizer.cpp:193 > +} else { > +usedDevices.erase(it++); > +} this is definitely wrong, erasing invalidates ite

D11745: databasesanitizer: Use flags for filtering

2018-04-12 Thread Stefan Brüns
bruns added a dependency: D12165: Use QStorageInfo instead of a homegrown implementation. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D11745 To: michaelh, #baloo, #frameworks, bruns Cc: bruns, ngraham, smithjd, ashaposhnikov, michaelh, astippich, spoorun, alexeymin

D12165: Use QStorageInfo instead of a homegrown implementation

2018-04-12 Thread Stefan Brüns
bruns added a dependent revision: D11745: databasesanitizer: Use flags for filtering. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12165 To: bruns, #baloo, michaelh Cc: #frameworks, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexeymin

D11452: sanitizer: Improve device listing

2018-04-12 Thread Stefan Brüns
bruns added a dependent revision: D12165: Use QStorageInfo instead of a homegrown implementation. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D11452 To: michaelh, #baloo, #frameworks, bruns Cc: ngraham, bruns, smithjd, ashaposhnikov, michaelh, astippich, spoorun, alex

D12165: Use QStorageInfo instead of a homegrown implementation

2018-04-12 Thread Stefan Brüns
bruns added a dependency: D11452: sanitizer: Improve device listing. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12165 To: bruns, #baloo, michaelh Cc: #frameworks, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexeymin

D11882: autotests: Use built executable instead of installed

2018-04-13 Thread Stefan Brüns
bruns added a comment. In D11882#239725 , @michaelh wrote: > In D11882#239638 , @alexeymin wrote: > > > Code looks fine, but I did not test if it actually works. ;) > > > It's a little hard to

D12135: Allow disabling of CoW to fail when not supported by filesystem

2018-04-13 Thread Stefan Brüns
bruns updated this revision to Diff 32074. bruns added a comment. rebase REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12135?vs=31946&id=32074 BRANCH master REVISION DETAIL https://phabricator.kde.org/D12135 AFFECTED FILES src/engine/fsutils.cpp s

D12135: Allow disabling of CoW to fail when not supported by filesystem

2018-04-13 Thread Stefan Brüns
bruns added a reviewer: michaelh. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12135 To: bruns, #baloo, adridg, #frameworks, michaelh Cc: adridg, #freebsd, #frameworks, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexeymin

D12165: Use QStorageInfo instead of a homegrown implementation

2018-04-13 Thread Stefan Brüns
bruns updated this revision to Diff 32078. bruns added a comment. rebase REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12165?vs=32033&id=32078 BRANCH master REVISION DETAIL https://phabricator.kde.org/D12165 AFFECTED FILES src/engine/experimental/da

D12165: Use QStorageInfo instead of a homegrown implementation

2018-04-13 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:0491ba6e8e96: Use QStorageInfo instead of a homegrown implementation (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12165?vs=32078&id=32087 REVI

D12005: Fix merging of terms in the AdvancedQueryParser

2018-04-13 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:c91c43393c33: Fix merging of terms in the AdvancedQueryParser (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12005?vs=31685&id=32089 REVISION DE

D12135: Allow disabling of CoW to fail when not supported by filesystem

2018-04-14 Thread Stefan Brüns
bruns added a comment. @michaelh can you accept - I can't ... REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12135 To: bruns, #baloo, adridg, #frameworks, michaelh Cc: adridg, #freebsd, #frameworks, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexey

D12192: Decode more documentUrls

2018-04-14 Thread Stefan Brüns
bruns requested changes to this revision. bruns added a comment. This revision now requires changes to proceed. Can you add a little bit more info to the commit message, e.g. an example how the output looks before and after? REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.o

D11753: baloodb: Add clean command

2018-04-14 Thread Stefan Brüns
bruns requested changes to this revision. bruns added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > databasesanitizer.cpp:166 > > -private: > Transaction* m_transaction; Why do you remove the private: here? > databasesanitizer.cpp:176 > +quint6

D11745: databasesanitizer: Use flags for filtering

2018-04-14 Thread Stefan Brüns
bruns accepted this revision. bruns added a comment. This revision is now accepted and ready to land. Save the missing comment for "tmpfs", looks fine, thanks! INLINE COMMENTS > databasesanitizer.cpp:250 > auto mounted = info.isValid(); > - > -if (missingOnly && mounted) { >

D12197: autotests: Test for multiple values

2018-04-14 Thread Stefan Brüns
bruns added a comment. "Prepare sample files for tags which may have multiple values" REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D12197 To: michaelh, #baloo, #frameworks, mgallien Cc: bruns, ashaposhnikov, michaelh, astippich, spoorun, ngraham, alexeymin

D10694: epubextractor: Handle multiple subjects better

2018-04-14 Thread Stefan Brüns
bruns added a comment. Sorry to join late here ... A QMap can store multiple values for one key, and a client reading the Map can use QMap::values() to get a list of all matching properties. If a client naively uses value() instead, it just gets the first value for the key, but so be it

D12135: Allow disabling of CoW to fail when not supported by filesystem

2018-04-15 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:92ff146f5563: Allow disabling of CoW to fail when not supported by filesystem (authored by bruns). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D12135?vs=32074&id=32211#toc REPOSITORY R293

D12136: Avoid hardcoding of filesystems supporting CoW

2018-04-15 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:5f3b8ead07eb: Avoid hardcoding of filesystems supporting CoW (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12136?vs=31944&id=32213 REVISION DET

<    1   2   3   4   5   6   7   8   9   10   >