Review Request 120132: correct documentation for overlays parameter

2014-09-10 Thread Stefan Brüns
--- The current documentation is incorrect, as no 'emblem' prefix is added to the overlay name. Also there is no mention how the emblems are placed. Instead of giving a complete definition, reference the drawOverlays method. Signed-off-by: Stefan Brüns stefan.bru...@rwth-aachen.de Diffs

Re: Review Request 120132: correct documentation for overlays parameter

2014-09-10 Thread Stefan Brüns
framework - Stefan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120132/#review66195 --- On Sept. 10, 2014, 9:24 p.m., Stefan Brüns

Re: Review Request 120132: correct documentation for overlays parameter

2014-09-11 Thread Stefan Brüns
the drawOverlays method. Signed-off-by: Stefan Brüns stefan.bru...@rwth-aachen.de Diffs - tier3/kiconthemes/src/kiconloader.h a32734564786ab1bd7920ce13339f7f8713d9260 Diff: https://git.reviewboard.kde.org/r/120132/diff/ Testing --- Thanks, Stefan Brüns

Re: Review Request 120132: correct documentation for overlays parameter

2014-09-11 Thread Stefan Brüns
On Sept. 10, 2014, 9:31 p.m., Luigi Toscano wrote: I guess this is about frameworks branch. If yes, please ignore it, as it has been split into the several framework repositories; check if the issue applies in the separate KIconThemes framework. Stefan Brüns wrote: patch applies

Review Request 120142: correct documentation for overlays parameter

2014-09-11 Thread Stefan Brüns
Brüns stefan.bru...@rwth-aachen.de Diffs - src/kiconloader.h 46d3017e703c38bfa4da9e5cb721b0c596a26815 Diff: https://git.reviewboard.kde.org/r/120142/diff/ Testing --- Thanks, Stefan Brüns ___ Kde-frameworks-devel mailing list Kde

Re: Review Request 120142: correct documentation for overlays parameter

2014-09-18 Thread Stefan Brüns
. Instead of giving a complete definition, reference the drawOverlays method. Signed-off-by: Stefan Brüns stefan.bru...@rwth-aachen.de Diffs - src/kiconloader.h 46d3017e703c38bfa4da9e5cb721b0c596a26815 Diff: https://git.reviewboard.kde.org/r/120142/diff/ Testing --- Thanks, Stefan

Review Request 120605: cleanup overlay icon usage

2014-10-16 Thread Stefan Brüns
-by: Stefan Brüns stefan.bru...@rwth-aachen.de Diffs - src/core/kfileitem.cpp 74dc069dc964f4fb0040a3dab916ff0d1e26602c Diff: https://git.reviewboard.kde.org/r/120605/diff/ Testing --- Patched kio, see BR for new screenshot Thanks, Stefan Brüns

Re: Review Request 120605: cleanup overlay icon usage

2014-10-18 Thread Stefan Brüns
? - Stefan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120605/#review68567 --- On Oct. 16, 2014, 3:32 p.m., Stefan Brüns wrote

Re: Review Request 120605: cleanup overlay icon usage

2014-11-01 Thread Stefan Brüns
--- On Oct. 16, 2014, 3:32 p.m., Stefan Brüns wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120605

Re: Review Request 120605: cleanup overlay icon usage

2014-11-10 Thread Stefan Brüns
with .gz file ending, there is a mimetype icon for gzip files. BUGS: 339193 Signed-off-by: Stefan Brüns stefan.bru...@rwth-aachen.de Diffs - src/core/kfileitem.cpp 74dc069dc964f4fb0040a3dab916ff0d1e26602c Diff: https://git.reviewboard.kde.org/r/120605/diff/ Testing --- Patched kio

Re: Review Request 122652: Use correct default value when UDS_ACCESS/UDS_FILE_TYPE is not set

2015-03-17 Thread Stefan Brüns
/kio/kfileitem.cpp f431d3608cfe646fb882365921e694af8ff8838f Diff: https://git.reviewboard.kde.org/r/122652/diff/ Testing --- dolphin remote: - no lock icon on smb:, mtp:, ... links Thanks, Stefan Brüns ___ Kde-frameworks-devel mailing list Kde

Review Request 123972: Clarify exit value for Unique instances

2015-05-31 Thread Stefan Brüns
--- The default exit value for a duplicate instance is 0, if not set from the already-running application. Signed-off-by: Stefan Brüns stefan.bru...@rwth-aachen.de Diffs - src/kdbusservice.h 8dd6357419c64e0b5a1eca6f3f56c760f8c398fa Diff: https://git.reviewboard.kde.org/r/123972

Re: Review Request 123972: Clarify exit value for Unique instances

2015-06-28 Thread Stefan Brüns
marked as submitted. Review request for KDE Frameworks and David Faure. Repository: kdbusaddons Description --- The default exit value for a duplicate instance is 0, if not set from the already-running application. Signed-off-by: Stefan Brüns stefan.bru...@rwth-aachen.de Diffs

Re: Review Request 124413: Enable PAM opening KWallet again

2015-07-21 Thread Stefan Brüns
On July 21, 2015, 3:57 p.m., Lamarque Souza wrote: src/runtime/kwalletd/main.cpp, line 113 https://git.reviewboard.kde.org/r/124413/diff/1/?file=386596#file386596line113 You should use strncmp instead of strcmp. Martin Klapetek wrote: Why would you think? The whole string is

Re: Review Request 125515: Preserve relative link targets when copying symlinks.

2015-10-22 Thread Stefan Brüns
> On Oct. 22, 2015, 4:32 p.m., Frank Reininghaus wrote: > > 4096 bytes looks reasonable. I think I would still find a fail-safe > > solution with a dynamically increasing buffer prettier, but it's so > > extremely unlikely that this will ever cause problems that it's not worth > > arguing

Re: Review Request 129720: [ExtractorCollection] Use mimetype inheritance to return plugins

2017-01-19 Thread Stefan Brüns
tps://git.reviewboard.kde.org/r/129720/#comment68240> This is broken: if (plugins.isEmpty()) { ... for (; it != d->m_extractors.constEnd(); it++) { if (plugins.contains( some_value ) The "plugins.contains(...)" is always false. - Stefan Brüns On

D5034: Add support for x-gvfs style options in fstab

2017-04-06 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > fstabdevice.cpp:47 > > -m_description = m_vendor + " on " + m_product; > +const QStringList = > FstabHandling::options(m_device).filter("x-gvfs-"); > + Why QStringList& instead of QStringList? > fstabdevice.cpp:51 > +if

D5034: Add support for x-gvfs style options in fstab

2017-07-11 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > fstabstorageaccess.cpp:48 > > +const QStringList = FstabHandling::options(device->device()); > +// GVFS by default doesn't show devices outside of /media, $HOME (and > some other locations) Another useless use of const QList&,

Re: Review Request 130058: Make kwalletd5 service both org.kde.kwalletd5 and org.kde.kwalletd

2017-05-27 Thread Stefan Brüns
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/130058/#review103250 --- Thanks for pushing this upstream! - Stefan Brüns On May

D7772: [solid/fstab] Swap vendor and product properties, allow i18n of description

2017-09-11 Thread Stefan Brüns
bruns created this revision. bruns added projects: Plasma, Frameworks. Restricted Application added subscribers: Frameworks, plasma-devel. REVISION SUMMARY The fstab backend exposes server and sharename as product and vendor properties, but had the meaning backwards (the product is

D7772: [solid/fstab] Swap vendor and product properties, allow i18n of description

2017-09-11 Thread Stefan Brüns
bruns abandoned this revision. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D7772 To: bruns, #plasma Cc: plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D7772: [solid/fstab] Swap vendor and product properties, allow i18n of description

2017-09-11 Thread Stefan Brüns
bruns removed a dependency: D7773: [solid/fstab] Swap vendor and product properties, allow i18n of description. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D7772 To: bruns, #plasma Cc: plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed,

D7773: [solid/fstab] Swap vendor and product properties, allow i18n of description

2017-09-11 Thread Stefan Brüns
bruns removed a dependent revision: D7772: [solid/fstab] Swap vendor and product properties, allow i18n of description. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D7773 To: bruns, #plasma Cc: plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed,

D7773: [solid/fstab] Swap vendor and product properties, allow i18n of description

2017-09-11 Thread Stefan Brüns
bruns added a dependent revision: D7774: [solid/fstab] Add support for x-gvfs style options in fstab. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D7773 To: bruns, #plasma Cc: plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg,

D7774: [solid/fstab] Add support for x-gvfs style options in fstab

2017-09-11 Thread Stefan Brüns
bruns added a dependency: D7773: [solid/fstab] Swap vendor and product properties, allow i18n of description. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D7774 To: bruns, #plasma Cc: plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed,

D7774: [solid/fstab] Add support for x-gvfs style options in fstab

2017-09-11 Thread Stefan Brüns
bruns created this revision. bruns added projects: Plasma, Frameworks. REVISION SUMMARY This fstab options allows an administrator to specify names and icons intended for the user, shown in a GUI For details, see https://git.gnome.org/browse/gvfs/tree/monitor/udisks2/what-is-shown.txt

D7773: [solid/fstab] Swap vendor and product properties, allow i18n of description

2017-09-11 Thread Stefan Brüns
bruns created this revision. bruns added projects: Plasma, Frameworks. Restricted Application added subscribers: Frameworks, plasma-devel. REVISION SUMMARY The fstab backend exposes server and sharename as product and vendor properties, but had the meaning backwards (the product is subordinate

D7772: [solid/fstab] Swap vendor and product properties, allow i18n of description

2017-09-11 Thread Stefan Brüns
bruns added a dependency: D7773: [solid/fstab] Swap vendor and product properties, allow i18n of description. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D7772 To: bruns, #plasma Cc: plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed,

D7773: [solid/fstab] Swap vendor and product properties, allow i18n of description

2017-09-11 Thread Stefan Brüns
bruns added a dependent revision: D7772: [solid/fstab] Swap vendor and product properties, allow i18n of description. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D7773 To: bruns, #plasma Cc: plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed,

D7774: [solid/fstab] Add support for x-gvfs style options in fstab

2017-09-12 Thread Stefan Brüns
bruns updated this revision to Diff 19458. bruns added a comment. Remove erroneous const, takeFirst requires a mutable list REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7774?vs=19418=19458 BRANCH master REVISION DETAIL

D7773: [solid/fstab] Swap vendor and product properties, allow i18n of description

2017-09-26 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R245:f299edfeb8fd: [solid/fstab] Swap vendor and product properties, allow i18n of description (authored by bruns). REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE

D7774: [solid/fstab] Add support for x-gvfs style options in fstab

2017-09-26 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R245:9929fc8fbdcb: [solid/fstab] Add support for x-gvfs style options in fstab (authored by bruns). REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7774?vs=19458=19949

D12696: Use the new uds implementation

2018-05-07 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > udsentry.cpp:132 > +} > +static void save(QDataStream , const UDSEntry ) > +{ Is there a reason to define the methods inside the class declaration? The diff would be significantly smaller if you kept the definition separate.

D12787: Ignore more types of source files

2018-05-14 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > ngraham wrote in fileexcludefilters.cpp:154 > My impression is that Baloo is really intended for user files; SVGs only get > their content indexed by accident, because they happen to be textual. I don't > think there's any textual content inside

D12787: Ignore more types of source files

2018-05-14 Thread Stefan Brüns
bruns added a comment. Does anyone know if there are any artifacts generated by the meson build system? INLINE COMMENTS > fileexcludefilters.cpp:74 > > // Compiled files > "*.class", // Java Probably `Bytecode` - we have `.o` above, which is also compiled >

D12787: Ignore more types of source files

2018-05-14 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > fileexcludefilters.cpp:154 > +"image/svg+xml", > +"image/svg+xml-compressed", > "application/x-awk", Hm, not to sure about this one - SVG typically has RDF metadata, and also everything in `` tags qualifies as "content". Do we have a

D12787: Ignore more types of source files

2018-05-14 Thread Stefan Brüns
bruns added a comment. If you want to read more about text in SVG: http://tavmjong.free.fr/blog/ To show a generalized XML extractor is sufficient for SVG: - Path data: `` - Single Line: `Single line` - Multiline: `This is some multiline Text` Non-text tags are empty

D12787: Ignore more types of source files

2018-05-14 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > fileexcludefilters.cpp:82 > +"*.jsc", // Javascript > +"Bytecode", > Thats not what I meant (I am not aware of anything generating a `Bytecode` file literally). I meant changing the `// Compiled files` comment to `// Bytecode files`,

D12696: Use the new uds implementation

2018-05-07 Thread Stefan Brüns
bruns added a comment. In D12696#259292 , @dfaure wrote: > I requested it, for proper encapsulation Whats the difference between class UDSEntryPrivate { void load(...) { // function body } }; and

D12696: Use the new uds implementation

2018-05-09 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > kfileitem.cpp:198 > const QByteArray pathBA = QFile::encodeName(path); > if (QT_LSTAT(pathBA.constData(), ) == 0) { > +m_entry.replace(KIO::UDSEntry::UDS_DEVICE_ID, > buf.st_dev);

D12696: Use the new uds implementation

2018-05-09 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > bruns wrote in kfileitem.cpp:198 > Wouldn't it be better to call m_entry.clear() and m_entry.insert(...) here? > > For the call from the constructor, m_entry is empty, so you save the > std::find_if() in the replace method calls. > > For any

D12335: Avoid infinite loops when fetching the URL from DocumentUrlDB

2018-04-27 Thread Stefan Brüns
bruns added a comment. Can someone please review this - would be nice to get rid of all the bug reports and //Baloo is shit, just disable it// "reviews" REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12335 To: bruns, #baloo, michaelh, #frameworks Cc: #frameworks,

D12578: fix some issues reported by clazy

2018-04-28 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > CMakeLists.txt:4 > set(REQUIRED_QT_VERSION 5.8.0) > -set(KF5_VERSION "5.45.0") # handled by release scripts > -set(KF5_DEP_VERSION "5.45.0") # handled by release scripts > +set(KF5_VERSION "5.44.0") # handled by release scripts >

D12197: autotests: Test for multiple values

2018-05-09 Thread Stefan Brüns
bruns requested changes to this revision. bruns added a comment. This revision now requires changes to proceed. This depends on the pending discussion how "Subject" and "Keyword" should be handled, see D10694 REPOSITORY R286 KFileMetaData REVISION

D10694: epubextractor: Handle multiple subjects better

2018-05-09 Thread Stefan Brüns
bruns requested changes to this revision. bruns added a comment. This revision now requires changes to proceed. Restricted Application edited subscribers, added: Baloo, kde-frameworks-devel; removed: Frameworks. Discussion regarding subject vs keywords is still pending, i.e. no conclusion.

D12114: Revive 'Description' property

2018-05-09 Thread Stefan Brüns
bruns added a comment. Restricted Application added a subscriber: kde-frameworks-devel. @michaelh - are you still working on this? I think using Property::Description for dc::description and the like is very sensible, so I would welcome if you pushed this forward. Regarding the

D8007: popplerextractor: don't try to guess the title if there isn't one.

2018-05-09 Thread Stefan Brüns
bruns accepted this revision. bruns added a comment. I think this is sensible, to much code for questionable results. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D8007 To: flameeyes, #frameworks, mgallien, bruns Cc: kde-frameworks-devel, #baloo, bruns,

D12801: search for album artist and albumartist tags in taglibextractor

2018-05-10 Thread Stefan Brüns
bruns accepted this revision. This revision is now accepted and ready to land. REPOSITORY R286 KFileMetaData BRANCH album_artist REVISION DETAIL https://phabricator.kde.org/D12801 To: astippich, mgallien, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich,

D12320: [RFC] add ability to read embedded cover files

2018-05-10 Thread Stefan Brüns
bruns requested changes to this revision. bruns added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > embeddedimagedata.cpp:69 > + > +if (types == EmbeddedImageData::FrontCover) { > + >

D12800: split tests for better readability

2018-05-10 Thread Stefan Brüns
bruns accepted this revision. bruns added a comment. This revision is now accepted and ready to land. Nice, thanks! REPOSITORY R286 KFileMetaData BRANCH split_tests REVISION DETAIL https://phabricator.kde.org/D12800 To: astippich, mgallien, bruns Cc: bruns, kde-frameworks-devel,

D10937: Retouching of Screen Layout Selection OSD Icons

2018-05-10 Thread Stefan Brüns
bruns added a comment. In D10937#260688 , @pstefan wrote: > It's supposed to be a projector screen. I took the original icon metaphor to be a projector screen, so I continued with that. I think you should remove the tripod - I have not

D10937: Retouching of Screen Layout Selection OSD Icons

2018-05-10 Thread Stefan Brüns
bruns added a comment. Two remarks regarding the rotation icons: 1. I would remove the keyboard from the icons - you will use these with a tablet, a 2-in-1, or a pivoted external display. 2. For the large versions, rotate the content pictogram/window not by ~20 degres, but ~70

D12886: check that ffmpeg is at least version 3.1 that introduce the API we require

2018-05-15 Thread Stefan Brüns
bruns accepted this revision. bruns added a comment. +1 REPOSITORY R286 KFileMetaData BRANCH master REVISION DETAIL https://phabricator.kde.org/D12886 To: mgallien, dfaure, michaelh, jriddell, bruns Cc: bruns, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich,

D12787: Ignore more types of source files

2018-05-15 Thread Stefan Brüns
bruns accepted this revision. bruns added a comment. This revision is now accepted and ready to land. Not tested by me, by looks good in general. REPOSITORY R293 Baloo BRANCH more-excluded-source-files (branched from master) REVISION DETAIL https://phabricator.kde.org/D12787 To:

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

2018-05-15 Thread Stefan Brüns
bruns added a comment. Restricted Application added a subscriber: kde-frameworks-devel. Kind request to review ... REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12047 To: bruns, #baloo, michaelh, ngraham, #frameworks Cc: kde-frameworks-devel, #frameworks,

D12233: Avoid manipulation of lists with quadratic complexity

2018-05-15 Thread Stefan Brüns
bruns added a comment. Restricted Application edited subscribers, added: kde-frameworks-devel; removed: Frameworks. Apparently @michaelh is MIA - how do we proceed here? REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12233 To: michaelh, #baloo, bruns Cc:

D12578: fix some issues reported by clazy

2018-05-15 Thread Stefan Brüns
bruns added a comment. Restricted Application added a subscriber: kde-frameworks-devel. @mgallien - I think this is trivial to fix up - can you do, so we have one less request open? REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12578 To: mgallien, #baloo,

D11282: less expensive findByUrl in KCoreDirListerCache

2018-05-17 Thread Stefan Brüns
bruns added a comment. Restricted Application added a subscriber: kde-frameworks-devel. @jtamate - can you use `arc --diff` next time you create a revision, instead of using the web upload? Not using arc results in phabricator being unable to show the context ("Context not available"),

D12578: fix some issues reported by clazy

2018-05-17 Thread Stefan Brüns
bruns requested changes to this revision. bruns added a comment. This revision now requires changes to proceed. One change pending, then you are good to go ... REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12578 To: mgallien, #baloo, #frameworks, bruns Cc:

D11282: less expensive findByUrl in KCoreDirListerCache

2018-05-17 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > dfaure wrote in kcoredirlister.cpp:852 > That was the case already hmm but not in my version of kio. It looks like > this patch is on top of other patches. D10742 - this makes it even harder to review ...

D12932: handle string lists as input

2018-05-16 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > result.cpp:90 > +const QStringList val = value.toStringList(); > +if (val.isEmpty()) > +return; this check is not strictly necessary - if the list is empty, you iterate zero times in the loop. But see below [1] >

D12578: fix some issues reported by clazy

2018-05-15 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > searchstore.cpp:163 > if (term.operation() == Term::And || term.operation() == Term::Or) { > QList subTerms = term.subTerms(); > QVector vec; `const QList subterms = ...` REPOSITORY R293 Baloo REVISION

D10742: get rid of the raw KFileItem pointers in KCoreDirListerCache

2018-05-23 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > kfileitem.cpp:1248 > +{ > +return d->m_hash < other.d->m_hash; > +} This is incomplete for two cases: 1. Same URL 2. Hash collision > kfileitem.h:490 > +/** > + * Returns -1 if other's URL is greater, 0 if == and 1 if less than (as >

D13018: Fix unit test for kfileplacesmodeltest

2018-05-22 Thread Stefan Brüns
bruns added a comment. Shouldn't this be handled by: http://doc.qt.io/qt-5/qstandardpaths.html#setTestModeEnabled REVISION DETAIL https://phabricator.kde.org/D13018 To: renatoo, ngraham Cc: bruns, ngraham, maximilianocuria, elvisangelaccio, kde-frameworks-devel, michaelh

D12578: fix some issues reported by clazy

2018-05-17 Thread Stefan Brüns
bruns accepted this revision. bruns added a comment. This revision is now accepted and ready to land. No, worries, just wanted to make shure this does not stall 1 meter to the finish line ... REPOSITORY R293 Baloo BRANCH arcpatch-D12578 REVISION DETAIL

D12932: handle string lists as input

2018-05-16 Thread Stefan Brüns
bruns accepted this revision. bruns added a comment. This revision is now accepted and ready to land. Output looks sane for me. REPOSITORY R293 Baloo BRANCH string_lists REVISION DETAIL https://phabricator.kde.org/D12932 To: astippich, bruns Cc: kde-frameworks-devel, #baloo,

D12932: handle string lists as input

2018-05-16 Thread Stefan Brüns
bruns added a comment. Can you do a manual test on an appropriate file, and add the output (before and after) for `balooshow -x `? REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12932 To: astippich, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh,

D12932: handle string lists as input

2018-05-16 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > result.cpp:90 > +bool shouldBeIndexed = > KFileMetaData::PropertyInfo(property).shouldBeIndexed(); > +for (const auto& val : value.toStringList()) > +{ `const auto list = value.toStringList();`, to avoid detach. >

D11365: also test for value types in taglibextractortest and fix errors

2018-05-16 Thread Stefan Brüns
bruns added inline comments. Restricted Application added a subscriber: kde-frameworks-devel. INLINE COMMENTS > mgallien wrote in taglibextractor.cpp:389 > Are you sure we need this cast ? I do not think we will ever need to have > negative track numbers added. The original type is unsigned int

D12932: handle string lists as input

2018-05-16 Thread Stefan Brüns
bruns added a comment. looks good, although untested REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12932 To: astippich, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns

D10742: get rid of the raw KFileItem pointers in KCoreDirListerCache

2018-05-23 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > kcoredirlister_p.h:302 > +// Remove the item from the sorted list (from the wrong place) and > insert it in the right place. > +void reinsert(KFileItem , const QUrl ) > +{ This can be better implemented with std::move and

D12320: [RFC] add ability to read embedded cover files

2018-05-23 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > embeddedimagedata.cpp:67 > + > +if (types & EmbeddedImageData::FrontCover || types & > EmbeddedImageData::AllImages) { > +imageData.insert(EmbeddedImageData::FrontCover, > d->getFrontCover(fileUrl,mimeType)); If you follow the

D11282: less expensive findByUrl in KCoreDirListerCache

2018-05-17 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > mwolff wrote in kcoredirlister.cpp:2532 > this can be slow, see above To make this more efficient, a little bit of reshuffling is needed IMHO: 1. Move the filter implementation from `KDL::Private::addNewItem(...)` to

D12696: Use the new uds implementation

2018-06-10 Thread Stefan Brüns
bruns added a comment. In D12696#276660 , @martinkostolny wrote: > `udsField=33554445, value=16384` `33554445 = 0x20d = UDS_NUMBER = 0x0200 | UDS_FILE_TYPE = 13`

D12320: add ability to read embedded cover files

2018-06-10 Thread Stefan Brüns
bruns accepted this revision. bruns added a comment. This revision is now accepted and ready to land. Thanks, good to go ... REPOSITORY R286 KFileMetaData BRANCH cover_read REVISION DETAIL https://phabricator.kde.org/D12320 To: astippich, mgallien, michaelh, bruns Cc: anthonyfieroni,

D12696: Use the new uds implementation

2018-06-10 Thread Stefan Brüns
bruns added a comment. In D12696#276696 , @bruns wrote: > `UDS_FILE_TYPE = 13`: 32768 -> 16384 (010 -> 04) > File mode changes from 'regular file' to 'directory'

D12696: Use the new uds implementation

2018-06-07 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > martinkostolny wrote in udsentry.cpp:106 > sftp slave crashes here when entering directory with links Any chance you can attach a debugger (or load the core file with gdb) and provide the values of 'udsField' and 'value'? REPOSITORY R241 KIO

D13216: Overhaul the file index scheduler.

2018-06-07 Thread Stefan Brüns
bruns added a comment. In D13216#275546 , @smithjd wrote: > In D13216#275538 , @bruns wrote: > > > @smithjd You have probaly become another victim of phabricator/arc. Your commits have been

D13216: Overhaul the file index scheduler.

2018-06-07 Thread Stefan Brüns
bruns added a comment. In D13216#275581 , @smithjd wrote: > According to the arc documentation, a --merge commit should land the commits separately. Still, **I** can not see the individual commits. REPOSITORY R293 Baloo REVISION

D13216: Overhaul the file index scheduler.

2018-06-07 Thread Stefan Brüns
bruns added a comment. @smithjd You have probaly become another victim of phabricator/arc. Your commits have been squashed by arc ... If you wan't your commits to stay separated, you have to do a `git checkout` of the first commit, do a `arc diff HEAD^1`, checkout the next commit, `arc

D13571: Correct KFormat::formatBytes examples

2018-06-18 Thread Stefan Brüns
bruns marked an inline comment as done. bruns added inline comments. INLINE COMMENTS > apol wrote in kformat.h:184 > What's "resp."? respectively REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D13571 To: bruns, #frameworks Cc: apol, kde-frameworks-devel,

D13571: Correct KFormat::formatBytes examples

2018-06-18 Thread Stefan Brüns
bruns marked an inline comment as done. bruns added a comment. In D13571#279453 , @apol wrote: > Then explain why it works? Actually the former behaviour is what I'd expect. The given example is just plain wrong. It does not work, it is

D13571: Correct KFormat::formatBytes examples

2018-06-16 Thread Stefan Brüns
bruns created this revision. bruns added a reviewer: Frameworks. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY The example left out the "precision" parameter,

D13571: Correct KFormat::formatBytes examples

2018-06-18 Thread Stefan Brüns
bruns added a comment. In D13571#279550 , @apol wrote: > > The given example is just plain wrong. It does not work, it is just some false prosa. > > Fair enough, yet the API user may want to be able to understand what's the logic behind it,

D13571: Correct KFormat::formatBytes examples

2018-06-18 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > kformattest.cpp:37 > QCOMPARE(format.formatByteSize(0), QStringLiteral("0 B")); > QCOMPARE(format.formatByteSize(50), QStringLiteral("50 B")); > QCOMPARE(format.formatByteSize(500), QStringLiteral("500 B")); See here for handling of

D13216: Overhaul the file index scheduler.

2018-06-18 Thread Stefan Brüns
bruns added a comment. This still contains two unrelated changes. Can you please submit each one individually? REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D13216 To: smithjd, bruns, mgallien Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh,

D13424: On config change halt the file indexer and run a file changed check when the file watches are updated.

2018-06-18 Thread Stefan Brüns
bruns added a comment. Two independent changes. Please provide a more useful commit message (for each one). REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D13424 To: smithjd, mgallien, bruns Cc: mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, michaelh,

D13425: Quit the file indexer when closing.

2018-06-18 Thread Stefan Brüns
bruns added a comment. Please provide a more specific commit message, i.e. what is "closing" The commit message should be understandable *without* reading the code change. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D13425 To: smithjd, bruns, mgallien Cc:

D13583: KFormat: Allow usage of quantities beyond bytes and seconds

2018-06-17 Thread Stefan Brüns
bruns created this revision. bruns added a reviewer: Frameworks. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY Its useful to allow automatic formatting of quantities

D13584: KFormat: Replace byte specific implementation with generic one

2018-06-17 Thread Stefan Brüns
bruns created this revision. bruns added a reviewer: Frameworks. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY There is no need for duplicate implementations. As

D13584: KFormat: Replace byte specific implementation with generic one

2018-06-17 Thread Stefan Brüns
bruns added a task: T8500: Add HIG rules about use of units/symbols in (config) UI. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D13584 To: bruns, #frameworks Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13583: KFormat: Allow usage of quantities beyond bytes and seconds

2018-06-17 Thread Stefan Brüns
bruns added a dependent revision: D13584: KFormat: Replace byte specific implementation with generic one. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D13583 To: bruns, #frameworks Cc: kde-frameworks-devel, astippich, michaelh, ngraham, bruns

D13584: KFormat: Replace byte specific implementation with generic one

2018-06-17 Thread Stefan Brüns
bruns added a dependency: D13583: KFormat: Allow usage of quantities beyond bytes and seconds. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D13584 To: bruns, #frameworks Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13583: KFormat: Allow usage of quantities beyond bytes and seconds

2018-06-17 Thread Stefan Brüns
bruns added a task: T8500: Add HIG rules about use of units/symbols in (config) UI. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D13583 To: bruns, #frameworks Cc: kde-frameworks-devel, astippich, michaelh, ngraham, bruns

D13460: implement the lyrics tag for taglibextractor

2018-06-17 Thread Stefan Brüns
bruns accepted this revision. This revision is now accepted and ready to land. REPOSITORY R286 KFileMetaData BRANCH lyrics REVISION DETAIL https://phabricator.kde.org/D13460 To: astippich, mgallien, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun,

D12233: Avoid manipulation of lists with quadratic complexity

2018-06-17 Thread Stefan Brüns
bruns added a comment. Still waiting for a final review ... REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12233 To: bruns, #baloo, michaelh Cc: dhaumann, ngraham, kde-frameworks-devel, jtamate, bruns, ashaposhnikov, michaelh, astippich, spoorun, abrahams

D13571: Correct KFormat::formatBytes examples

2018-06-19 Thread Stefan Brüns
bruns added a comment. In D13571#280135 , @apol wrote: > In D13571#279554 , @bruns wrote: > > > In D13571#279550 , @apol wrote: > > > > > > The given

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

2018-05-28 Thread Stefan Brüns
bruns added a comment. If there is noone willing to review, I will push this tomorrow REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12047 To: bruns, #baloo, michaelh, ngraham, #frameworks Cc: kde-frameworks-devel, #frameworks, ashaposhnikov, michaelh, astippich,

D12233: Avoid manipulation of lists with quadratic complexity

2018-05-28 Thread Stefan Brüns
bruns updated this revision to Diff 35068. bruns added a comment. Fix remaining issue (inverse condition in std::partition), cleanup REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12233?vs=32595=35068 BRANCH arcpatch-D12233 REVISION DETAIL

D12233: Avoid manipulation of lists with quadratic complexity

2018-05-28 Thread Stefan Brüns
bruns commandeered this revision. bruns edited reviewers, added: michaelh; removed: bruns. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12233 To: bruns, #baloo, michaelh Cc: ngraham, kde-frameworks-devel, jtamate, bruns, ashaposhnikov, michaelh, astippich, spoorun

  1   2   3   4   5   6   7   8   9   10   >