D17816: Support for xattrs on kio copy/move

2021-03-30 Thread Stefan Brüns
bruns added a comment. In D17816#677552 , @ngraham wrote: > What is the status of this? How do we move forwards? I know this has gone on a long time and we're all getting tired, but I think we can push this past the finish line without too much

D17816: Support for xattrs on kio copy/move

2021-03-02 Thread Stefan Brüns
bruns added a comment. In D17816#677455 , @kdudka wrote: > @bruns I find your attitude unnecessarily hostile. If you think that the code is perfect as it is, feel free to patch it case by case until it eventually works for everybody. That is

D17816: Support for xattrs on kio copy/move

2021-03-02 Thread Stefan Brüns
bruns added a comment. In D17816#677453 , @kdudka wrote: > I do not think we have to. Please have a look at how attr_copy_fd() from is implemented: http://git.savannah.nongnu.org/cgit/attr.git/tree/libattr/attr_copy_fd.c?id=a4187bec#n111

D17816: Support for xattrs on kio copy/move

2021-03-02 Thread Stefan Brüns
bruns added a comment. In D17816#677451 , @kdudka wrote: > The man page says that one has to check return value of the second call. It does not say that the function needs to be called in a loop indefinitely until it succeeds. And if

D17816: Support for xattrs on kio copy/move

2021-03-02 Thread Stefan Brüns
bruns added a comment. In D17816#677449 , @kdudka wrote: > Even after applying the proposed patch, the code still looks problematic to me. I would prefer to have it explained first. When fgetxattr(..., 0) returns -1/ERANGE, what is the point

D17816: Support for xattrs on kio copy/move

2020-10-26 Thread Stefan Brüns
bruns added a comment. Looks ok to me now. Can someone else please double check? REVISION DETAIL https://phabricator.kde.org/D17816 To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann,

D23694: Add support for sshfs to the fstab backend

2020-10-24 Thread Stefan Brüns
bruns accepted this revision. This revision is now accepted and ready to land. REPOSITORY R245 Solid BRANCH master REVISION DETAIL https://phabricator.kde.org/D23694 To: lbeltrame, bruns, broulik, fvogt, #kde_connect Cc: elvisangelaccio, albertvaka, ngraham, kde-frameworks-devel,

D23694: Add support for sshfs to the fstab backend

2020-10-15 Thread Stefan Brüns
bruns added a comment. One possibility to hide it would be the `x-gvfs-*` options, though I am not sure if it is possible to pass these to fuse. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D23694 To: lbeltrame, bruns, broulik, fvogt, #kde_connect Cc:

D17816: Support for xattrs on kio copy/move

2020-09-17 Thread Stefan Brüns
bruns requested changes to this revision. bruns added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > jobtest.cpp:471 > + > +void JobTest::setXattr(const QString ) > +{ should be `dest`. > jobtest.cpp:489 > +qDebug() << resultdest; > +

D17816: Support for xattrs on kio copy/move

2020-09-10 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > bruns wrote in jobtest.cpp:780 > This will set `m_SkipXattr` unconditionally even if the source FS does not > support Xattrs. And now you **only** check the source FS, but no longer the destination FS. REVISION DETAIL

D17816: Support for xattrs on kio copy/move

2020-09-08 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > jobtest.cpp:467 > +path.removeLast(); > +QString writeTest = path.join("/") + "/fsXattrTestFile"; > +createTestFile(writeTest); const QString And if you just call this with the target directory (e.g. ` homeTmpDir()`), you can skip the

D29526: Thumbnails: make thumbnail generation dpr-aware

2020-09-08 Thread Stefan Brüns
bruns added a comment. In D29526#676402 , @meven wrote: > In D29526#676386 , @bruns wrote: > > > For all but text the DPR is completely irrelevant, large@1 is identical to normal@2. > > > Yes

D29397: KPreviewJob : Support for DeviceRatioPixel

2020-09-07 Thread Stefan Brüns
bruns added a comment. In D29397#663902 , @broulik wrote: > > At the risk of asking a stupid question, why? > > The text thumbnailer should be able to produce readable text on high dpi, or the folder thumbnailer should render the folder icon

D29526: Thumbnails: make thumbnail generation dpr-aware

2020-09-07 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > imagecreator.cpp:48 > +ir.setQuality(75); // set quality so that the jpeg handler will use a > high quality downscaler > + > img = ir.read(); This whole hunk should not be part of this submission. Also, the comment ("jpeg") is likely

D29526: Thumbnails: make thumbnail generation dpr-aware

2020-09-07 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > thumbnail.cpp:143 > > +bool createThumbnail(ThumbCreator *creator, const QString , int width, > int height, QImage , qreal devicePixelRatio = 1.0) > +{ Has to be static or in an anonymous namespace. > thumbnail.cpp:149 > +} else { > +

D29526: Thumbnails: make thumbnail generation dpr-aware

2020-09-07 Thread Stefan Brüns
bruns requested changes to this revision. bruns added a comment. This revision now requires changes to proceed. For all but text the DPR is completely irrelevant, large@1 is identical to normal@2. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D29526 To: meven,

D29397: KPreviewJob : Support for DeviceRatioPixel

2020-09-07 Thread Stefan Brüns
bruns added a comment. Most thumbnailers are completely DPR agnostic, and will create identical thumbnails for large@1 and normal@2. Having both is a waste of disk space and CPU time. The thumbnailer should have a key in its metadata to tell if it depends on the DPR, and only then

D28745: Skip caching thumbnails on encrypted filesystems

2020-09-04 Thread Stefan Brüns
bruns added a comment. In D28745#676320 , @marcingu wrote: > In D28745#676317 , @bruns wrote: > > > In D28745#676313 , @marcingu wrote: > > > > > Ping!

D28745: Skip caching thumbnails on encrypted filesystems

2020-09-04 Thread Stefan Brüns
bruns added a comment. In D28745#676313 , @marcingu wrote: > Ping! > I'm remanding about question early, because I could do much more work if I get to do it on weekend. > > Question: > This code won't save thumbnail for file on any

D28745: Skip caching thumbnails on encrypted filesystems

2020-09-01 Thread Stefan Brüns
bruns requested changes to this revision. bruns added a comment. This revision now requires changes to proceed. @marcingu - have you even verified this works? - I am quite sure it does not, neither for fuse.encfs, fuse.cryfs (as used by Vaults), nor for any LUKS encrypted devices.

D17816: Support for xattrs on kio copy/move

2020-08-26 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > file_unix.cpp:583 > +qCDebug(KIO_FILE) << "the file don't have any xattr"; > +return false; > +} Should be `true` REVISION DETAIL https://phabricator.kde.org/D17816 To: arrowd, dfaure, chinmoyr, bruns,

D28745: Skip caching thumbnails on encrypted filesystems

2020-08-24 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > thumbnail.cpp:62 > +#include > +#include > What are all these includes for? > thumbnail.cpp:563 > > -if (validThumbnails > 0 && hadFirstThumbnail == dir.filePath()) { > +if (validThumbnails > 0 && hadFirstThumbnail ==

D28745: Skip caching thumbnails on encrypted filesystems

2020-06-30 Thread Stefan Brüns
bruns added a comment. In D28745#675698 , @marcingu wrote: > > How often do I have to repeat the thumbnailer has to use the canonical path anyway? Please use that. > > `storageAccessFromPath` converts given path into canonical. I figured it

D17816: Support for xattrs on kio copy/move

2020-06-29 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > bruns wrote in jobtest.cpp:487 > this obviously needs test cases with a key ley/value len > 512, to check the > the array-to-short/resize path. not done ... > file_unix.cpp:620 > +#endif > +qDebug(KIO_FILE) << valuelen << " " << key

D28745: Skip caching thumbnails on encrypted filesystems

2020-06-29 Thread Stefan Brüns
bruns added a comment. In D28745#675685 , @marcingu wrote: > Solid::Device device = Solid::Device::storageAccessFromPath(filePath); > if (device.is()) { > allowCache = device.as()->usage() !=

D17816: Support for xattrs on kio copy/move

2020-06-24 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > arrowd wrote in file_unix.cpp:625 > Why? `ERANGE` means we need to come up with new value for `valuelen`, so we > set it to zero and start over. On the new iteration it gets passed into > `fgetxattr`/`extattr_get_fd` to find out sufficient value.

D17816: Support for xattrs on kio copy/move

2020-06-22 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > file_unix.cpp:580 > +if (listlen == 0) { > +qCDebug(KIO_FILE) << "file" << src_fd << "don't have any xattr"; > +return false; `src_fd` is quite meaningless as debug output > file_unix.cpp:625 > +

D29207: [Indexers] Ignore name-based mimetype for initial indexing decisions

2020-06-10 Thread Stefan Brüns
bruns closed this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D29207 To: bruns, #baloo, ngraham Cc: kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, fbampaloukas, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D28745: Skip caching thumbnails on encrypted filesystems

2020-06-10 Thread Stefan Brüns
bruns added a comment. In D28745#674863 , @meven wrote: > In D28745#674827 , @bruns wrote: > > > In D28745#674711 , @meven wrote: > > > > > > > > >

D29207: [Indexers] Ignore name-based mimetype for initial indexing decisions

2020-06-10 Thread Stefan Brüns
bruns edited the summary of this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D29207 To: bruns, #baloo, ngraham Cc: kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, fbampaloukas, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham,

D28745: Skip caching thumbnails on encrypted filesystems

2020-06-07 Thread Stefan Brüns
bruns added a comment. And can you please use arc to upload the patch - it is nearly impossible to do a review with the missing context REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D28745 To: marcingu, ivan, broulik, #dolphin, ngraham, meven, bruns Cc: thiago,

D28745: Skip caching thumbnails on encrypted filesystems

2020-06-07 Thread Stefan Brüns
bruns requested changes to this revision. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D28745 To: marcingu, ivan, broulik, #dolphin, ngraham, meven, bruns Cc: thiago, bruns, meven, ngraham, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, pberestov,

D28745: Skip caching thumbnails on encrypted filesystems

2020-06-07 Thread Stefan Brüns
bruns added a comment. In D28745#674711 , @meven wrote: > > Solid does not provide straight `folder => StorageVolume` yet, but I think Solid could have such a utility feature added. > Something like

D17816: Support for xattrs on kio copy/move

2020-06-02 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > file_unix.cpp:142 > +ssize_t listlen = 0; > +QByteArray keylist(listlen, Qt::Uninitialized); > +do { just `QByteArray keylist;` > file_unix.cpp:143 > +QByteArray keylist(listlen, Qt::Uninitialized); > +do { > +

D17816: Support for xattrs on kio copy/move

2020-06-01 Thread Stefan Brüns
bruns requested changes to this revision. bruns added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > file_unix.cpp:206 > +if (valuelen == -1 && errno == ERANGE) { > +value.resize(valuelen + 512); > +} WRONG WRONG WRONG

D17816: Support for xattrs on kio copy/move

2020-05-27 Thread Stefan Brüns
bruns requested changes to this revision. bruns added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > jobtest.cpp:487 > +attrs["user.fattr.with.a.lot.of.namespaces"] = "true"; > +attrs["user.name with space"] = "value with spaces"; > +return attrs;

D17816: Support for xattrs on kio copy/move

2020-05-25 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > jobtest.cpp:502 > +// arguments 0:"-n" 1:"name" 2:"-v", 3:"value" 4:src > +arguments = QStringList {"-n", "", "-v", "", "-h", src}; > +keyIndex = 1; `std::function m_setXattrFormatArgs;` ... m_setXattrFormatArgs =

D28590: Add a QString Solid::Device::displayName, used in Fstab Device for network mounts

2020-05-24 Thread Stefan Brüns
bruns accepted this revision. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D28590 To: meven, #frameworks, bruns, sitter, dfaure, ngraham Cc: ngraham, dfaure, broulik, ervin, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns

D29810: Don't use the setenv function after fork

2020-05-23 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > kcrash.cpp:689 > { > +static char autorestarted_envvar[] = "KCRASH_AUTO_RESTARTED"; > +char** environ_end; move this to the lambda scope, you can also make the var name shorter then. > kcrash.cpp:698 > +} > +auto end =

D29597: file extractor: fix linking

2020-05-10 Thread Stefan Brüns
bruns requested changes to this revision. bruns added a comment. This revision now requires changes to proceed. Your build system seems to be broken. The "missing" methods are statically linked, see src/file/extractor/CMakeFiles/baloo_file_extractor.dir/baloosettings.cpp.o REPOSITORY R293

D28745: Skip caching thumbnails on encrypted filesystems

2020-05-09 Thread Stefan Brüns
bruns added a comment. In D28745#667225 , @thiago wrote: > Use Solid and ask it if the device is on an encrypted device. If Solid does not have such an API, add it. I fully second that. REPOSITORY R320 KIO Extras REVISION DETAIL

D28745: Skip caching thumbnails on encrypted filesystems

2020-05-09 Thread Stefan Brüns
bruns added a comment. I think the patch as is has some structural problems. I think it would be significantly cleaner when: - the "cacheAllowed" checking code is moved into a separate function - the global `allowCache` variable is removed, and the value is passed into

D29454: [TaglibExtractor] Add support for Audible "Enhanced Audio" audio books

2020-05-05 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R286:0975ed45af79: [TaglibExtractor] Add support for Audible Enhanced Audio audio books (authored by bruns). REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE

D29454: [TaglibExtractor] Add support for Audible "Enhanced Audio" audio books

2020-05-05 Thread Stefan Brüns
bruns added a comment. In D29454#664222 , @ngraham wrote: > Nice! and when you combine it with https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/577, Elisa can play Audible audio books ;-) REPOSITORY R286

D29454: [TaglibExtractor] Add support for Audible "Enhanced Audio" audio books

2020-05-05 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Frameworks, Baloo, ngraham, astippich, mgallien. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY Audible AAX files are standard ISO Base Media

D28932: Store filename terms just once

2020-05-04 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:7605f4d7f7c4: Store filename terms just once (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28932?vs=80439=81937 REVISION DETAIL

D28932: Store filename terms just once

2020-05-04 Thread Stefan Brüns
bruns added a comment. This has been pending for more than two weeks now, without any sort of review ... @ngraham If you have any questions, please ask! REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D28932 To: bruns, #baloo, ngraham Cc: kde-frameworks-devel,

D28932: Store filename terms just once

2020-05-02 Thread Stefan Brüns
bruns edited the summary of this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D28932 To: bruns, #baloo, ngraham Cc: kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, fbampaloukas, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham,

D28932: Store filename terms just once

2020-05-02 Thread Stefan Brüns
bruns added a comment. Ping! REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D28932 To: bruns, #baloo, ngraham Cc: kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, fbampaloukas, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns,

D29188: [FileWatch] Remove redundant watchIndexedFolders() slot

2020-04-27 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:1793785420b7: [FileWatch] Remove redundant watchIndexedFolders() slot (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29188?vs=81201=81347

D29199: honor the extractMetaData flag

2020-04-26 Thread Stefan Brüns
bruns accepted this revision. This revision is now accepted and ready to land. REPOSITORY R286 KFileMetaData BRANCH honorFlag REVISION DETAIL https://phabricator.kde.org/D29199 To: astippich, #baloo, bruns, ngraham Cc: kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack,

D29207: [Indexers] Ignore name-based mimetype for initial indexing decisions

2020-04-26 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, ngraham. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY The name based mime type is inaccurate, so it should not be used to decide

D28932: Store filename terms just once

2020-04-26 Thread Stefan Brüns
bruns added a dependent revision: D29207: [Indexers] Ignore name-based mimetype for initial indexing decisions. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D28932 To: bruns, #baloo, ngraham Cc: kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack,

D29199: honor the extractMetaData flag

2020-04-26 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > exiv2extractor.cpp:172 > > -if (image->pixelHeight()) { > -result->add(Property::Height, image->pixelHeight()); > -} > +if (result->inputFlags() & ExtractionResult::ExtractMetaData) { > `if (!) return` > astippich wrote

D29191: [FileWatch] Fix watch updates on config changes

2020-04-26 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:c76397081d1e: [FileWatch] Fix watch updates on config changes (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29191?vs=81206=81243 REVISION

D29189: [KInotify] Fix path matching when removing watches

2020-04-26 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:73183acf00a2: [KInotify] Fix path matching when removing watches (authored by bruns). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D29189?vs=81204=81242#toc REPOSITORY R293 Baloo CHANGES

D29199: honor the extractMetaData flag

2020-04-26 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > office2007extractor.cpp:79 > + > +if (docPropsEntries.contains(QStringLiteral("core.xml")) && > result->inputFlags() & ExtractionResult::ExtractMetaData) { > QDomDocument coreDoc(QStringLiteral("core")); Long line bool

D29199: honor the extractMetaData flag

2020-04-26 Thread Stefan Brüns
bruns requested changes to this revision. This revision now requires changes to proceed. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D29199 To: astippich, #baloo, bruns, ngraham Cc: kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, fbampaloukas,

D29189: [KInotify] Fix path matching when removing watches

2020-04-26 Thread Stefan Brüns
bruns edited the summary of this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D29189 To: bruns, #baloo, ngraham Cc: kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, fbampaloukas, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham,

D29189: [KInotify] Fix path matching when removing watches

2020-04-26 Thread Stefan Brüns
bruns added a dependent revision: D29191: [FileWatch] Fix watch updates on config changes. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D29189 To: bruns, #baloo, ngraham Cc: kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, fbampaloukas, domson,

D29191: [FileWatch] Fix watch updates on config changes

2020-04-26 Thread Stefan Brüns
bruns edited the summary of this revision. bruns added a dependency: D29189: [KInotify] Fix path matching when removing watches. REPOSITORY R293 Baloo BRANCH submit REVISION DETAIL https://phabricator.kde.org/D29191 To: bruns, #baloo, ngraham, meven Cc: meven, kde-frameworks-devel,

D29190: [FileWatchTest] Extend coverage to config updates

2020-04-26 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:1a3c23f076d2: [FileWatchTest] Extend coverage to config updates (authored by bruns). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D29190?vs=81207=81229#toc REPOSITORY R293 Baloo CHANGES

D29190: [FileWatchTest] Extend coverage to config updates

2020-04-26 Thread Stefan Brüns
bruns marked an inline comment as done. REPOSITORY R293 Baloo BRANCH fwtest REVISION DETAIL https://phabricator.kde.org/D29190 To: bruns, #baloo, ngraham, meven Cc: meven, kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, fbampaloukas, domson, ashaposhnikov, michaelh,

D29190: [FileWatchTest] Extend coverage to config updates

2020-04-26 Thread Stefan Brüns
bruns marked an inline comment as done. bruns added inline comments. INLINE COMMENTS > meven wrote in filewatchtest.cpp:223 > Maybe less, to keep the test reasonably fast. Does not really matter as soon as the code is fixed (review pending). Default would be 5000. REPOSITORY R293 Baloo

D29190: [FileWatchTest] Extend coverage to config updates

2020-04-25 Thread Stefan Brüns
bruns updated this revision to Diff 81207. bruns added a comment. whitespace REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29190?vs=81205=81207 BRANCH fwtest REVISION DETAIL https://phabricator.kde.org/D29190 AFFECTED FILES

D29190: [FileWatchTest] Extend coverage to config updates

2020-04-25 Thread Stefan Brüns
bruns added a dependent revision: D29191: [FileWatch] Fix watch updates on config changes. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D29190 To: bruns, #baloo, ngraham Cc: kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, fbampaloukas, domson,

D29191: [FileWatch] Fix watch updates on config changes

2020-04-25 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, ngraham. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY Track the current list of include/exclude folders, and on a config change

D29190: [FileWatchTest] Extend coverage to config updates

2020-04-25 Thread Stefan Brüns
bruns added a dependent revision: D29189: [KInotify] Fix path matching when removing watches. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D29190 To: bruns, #baloo, ngraham Cc: kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, fbampaloukas, domson,

D29189: [KInotify] Fix path matching when removing watches

2020-04-25 Thread Stefan Brüns
bruns edited the summary of this revision. bruns added a dependency: D29190: [FileWatchTest] Extend coverage to config updates. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D29189 To: bruns, #baloo, ngraham Cc: kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n,

D29190: [FileWatchTest] Extend coverage to config updates

2020-04-25 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, ngraham. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY Currently, config changes are not covered in the tests, add a test case

D29189: [KInotify] Fix path matching when removing watches

2020-04-25 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, ngraham. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY As in removeWatch the path is passed in with a trailing slash, no path is

D29188: [FileWatch] Remove redundant watchIndexedFolders() slot

2020-04-25 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, ngraham. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY updateIndexedFoldersWatches() has the same effect, so there is no need for

D29181: Use KFileMetaData for XAttr support instead of private reimplementation

2020-04-25 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:774bd228b749: Use KFileMetaData for XAttr support instead of private reimplementation (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE

D29138: [Solid] Replace foreach (deprecated) with range/index for

2020-04-25 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > ahmadsamir wrote in fakeopticaldisc.cpp:43 > I was talking about something like Solid::OpticalDrive::MediumType(0); > ContentType has Solid::OpticalDisc::NoContent whereas MediumType has nothing > similar. > > IIUC named casts are preferred over

D29181: Use KFileMetaData for XAttr support instead of private reimplementation

2020-04-25 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, ngraham. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY This removes a lot of duplicate code, and on top it actually works as it no

D28980: Revert "add Baloo DBus signals for moved or removed files"

2020-04-25 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:c2865b207907: Revert add Baloo DBus signals for moved or removed files (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28980?vs=80540=81180

D28956: [QML Monitor] Show remaining time as soon as possible

2020-04-25 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:60766d57999c: [QML Monitor] Show remaining time as soon as possible (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28956?vs=80489=81179

D29138: [Solid] Replace foreach (deprecated) with range/index for

2020-04-25 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > ahmadsamir wrote in fakeopticaldisc.cpp:43 > It doesn't make the code more readable; and using QMap::constFind() while > iterating is more recognizable/widely-used. You are contradicting yourself. You use value() for content, but for

D28932: Store filename terms just once

2020-04-25 Thread Stefan Brüns
bruns added a comment. In D28932#657011 , @ngraham wrote: > I'll get around to reviewing this soon. I'm trying to figure out of I think the loss is acceptable. There is no loss, there is even a gain (queries work correctly in all

D28776: FstabDevice: Avoid recurrent construction of emblems QStringList

2020-04-25 Thread Stefan Brüns
bruns added a comment. Ping! REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D28776 To: bruns, #frameworks, apol Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28777: FstabDevice: Reevaluate emblems only when state changes

2020-04-25 Thread Stefan Brüns
bruns added a comment. Ping! REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D28777 To: bruns, #frameworks Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28956: [QML Monitor] Show remaining time as soon as possible

2020-04-25 Thread Stefan Brüns
bruns added a comment. Ping! REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D28956 To: bruns, #baloo, ngraham Cc: apol, kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, fbampaloukas, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns,

D28932: Store filename terms just once

2020-04-25 Thread Stefan Brüns
bruns added a comment. Ping! REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D28932 To: bruns, #baloo, ngraham Cc: kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, fbampaloukas, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns,

D29138: [Solid] Replace foreach (deprecated) with range/index for

2020-04-24 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > ahmadsamir wrote in fakeopticaldisc.cpp:43 > /* brown paper bag */, removed the lines. > > I don't see a default a-la-NoContent MediumType member. Just use `0`. REPOSITORY R245 Solid BRANCH l-foreach (branched from master) REVISION DETAIL

D29138: [Solid] Replace foreach (deprecated) with range/index for

2020-04-24 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > ahmadsamir wrote in fakeopticaldisc.cpp:43 > Nice. :) > > I think Solid::OpticalDisc::NoContent is more readable. You forgot to remove the old lines. Can also be applied to supportedMedia REPOSITORY R245 Solid BRANCH l-foreach (branched

D29138: [Solid] Replace foreach (deprecated) with range/index for

2020-04-24 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > fakeopticaldisc.cpp:43 > } > } > These four lines can be written as `content |= map.value(type, Solid::OpticalDisc::ContentType(0));` REPOSITORY R245 Solid BRANCH l-foreach (branched from master) REVISION DETAIL

D28590: Add a QString Solid::Device::displayName, used in Fstab Device for network mounts

2020-04-24 Thread Stefan Brüns
bruns added a comment. In D28590#656133 , @meven wrote: > In D28590#654139 , @bruns wrote: > > > Do not create m_storageAccess in the constructor > > > Hmm, you told me `the f

D29138: [Solid] Replace foreach (deprecated) with range/index for

2020-04-23 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > fakeopticaldisc.cpp:38 > + > +for (auto it = map.cbegin(); it != map.cend(); ++it) { > +if (content_typelist.contains(it.value())) { dito REPOSITORY R245 Solid BRANCH l-foreach (branched from master) REVISION DETAIL

D29138: [Solid] Replace foreach (deprecated) with range/index for

2020-04-23 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > fakecdrom.cpp:48 > + > +for (auto it = map.cbegin(); it != map.cend(); ++it) { > +if (supported_medialist.contains(it.value())) { The obviously better solution is to iterate over `supported_medialist` and swap the key and value of the

D29095: New much simpler mouse icon that works in both light and dark breeze

2020-04-23 Thread Stefan Brüns
bruns added a comment. > Bruns's comment about right-handed mice (gradient) is due to the quite asymmetrical shape many modern mice use for ergonomic reasons (RSI). No, even for a ball bottom right is darkest. Shapes should be drawn as if there were a light at the top left, i.e. with a

D29095: New much simpler mouse icon that works in both light and dark breeze

2020-04-23 Thread Stefan Brüns
bruns added a comment. F8255905: bitmap.png REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D29095 To: saligari, #vdg Cc: bruns, ouwerkerk, ndavis, ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh

D29095: New much simpler mouse icon that works in both light and dark breeze

2020-04-23 Thread Stefan Brüns
bruns added a comment. The new one looks very oldschool. Some ideas how to improve that: - buttons are typically only separated from each other, but not from the body, try removing the horizontal line - the body should be narrower at the center - the gradient does not match the

D29034: Add systemd user service file for kded

2020-04-23 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > davidedmundson wrote in plasma-kded.service.in:5 > @CMAKE_INSTALL_FULL_BINDIR@ How about using some LIBEXEC dir instead, this should never be called directly, or am I missing something? REPOSITORY R297 KDED REVISION DETAIL

D27152: Introduce FilesystemEntry class

2020-04-22 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > hallas wrote in fstabhandling.cpp:374 > @bruns - I am little confused now. You wrote previously that you could see > the point in the `id()` function, but now you want it moved? > What kind of unit test are you missing? I can't quite follow the

D28590: Add a QString Solid::Device::displayName, used in Fstab Device for network mounts

2020-04-21 Thread Stefan Brüns
bruns requested changes to this revision. bruns added a comment. This revision now requires changes to proceed. Do not create m_storageAccess in the constructor REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D28590 To: meven, #frameworks, bruns, sitter Cc:

D28780: [FstabWatcher] Fix loosing of fstab watcher

2020-04-21 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R245:5f97573881fe: [FstabWatcher] Fix loosing of fstab watcher (authored by bruns). REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28780?vs=79954=80741 REVISION DETAIL

D28776: FstabDevice: Avoid recurrent construction of emblems QStringList

2020-04-20 Thread Stefan Brüns
bruns added a comment. Ping! REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D28776 To: bruns, #frameworks, apol Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28780: [FstabWatcher] Fix loosing of fstab watcher

2020-04-20 Thread Stefan Brüns
bruns added a reviewer: ngraham. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D28780 To: bruns, #frameworks, ngraham Cc: apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28780: [FstabWatcher] Fix loosing of fstab watcher

2020-04-20 Thread Stefan Brüns
bruns marked an inline comment as done. bruns added a comment. Ping! REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D28780 To: bruns, #frameworks, ngraham Cc: apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28995: Add imperial gallon and US pint

2020-04-20 Thread Stefan Brüns
bruns added a comment. In D28995#652764 , @ngraham wrote: > Aren't Imperial and US the same thing?The USA is the only country that still officially uses the imperiaI system. Every time you use "is the same as" in reference to imperial

  1   2   3   4   5   6   7   8   9   10   >