D14121: KFormat: Add unit tests for mili/micro SI prefixes

2018-07-15 Thread Stefan Brüns
bruns marked an inline comment as done. bruns added inline comments. INLINE COMMENTS > astippich wrote in kformattest.cpp:118 > typo: KFormat::UnitPrefix::Mil_l_i . Should be safe to change it since it > hasn't been released in its current state, isn't it? see D14135

D14135: KFormat: fix typo in SI prefix name enum

2018-07-15 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R244:2300c57fcd75: KFormat: fix typo in SI prefix name enum (authored by bruns). REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14135?vs=37807&id=37816 REVISION DET

D14121: KFormat: Add unit tests for mili/micro SI prefixes

2018-07-15 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. bruns marked an inline comment as done. Closed by commit R244:b95bfd7e47a3: KFormat: Add unit tests for mili/micro SI prefixes (authored by bruns). REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabrica

D14122: KFormat: Replace unicode literal with unicode codepoint to fix MSVC build

2018-07-15 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R244:5c3f5eade4cc: KFormat: Replace unicode literal with unicode codepoint to fix MSVC build (authored by bruns). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D14122?vs=37776&id=37819#toc REPOSITO

D12233: Avoid manipulation of lists with quadratic complexity

2018-07-15 Thread Stefan Brüns
bruns added a reviewer: Frameworks. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12233 To: bruns, #baloo, michaelh, mgallien, mpyne, #frameworks Cc: mpyne, dhaumann, ngraham, kde-frameworks-devel, jtamate, bruns, ashaposhnikov, michaelh, astippich, spoorun, abrahams

D12233: Avoid manipulation of lists with quadratic complexity

2018-07-15 Thread Stefan Brüns
bruns marked an inline comment as done. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12233 To: bruns, #baloo, michaelh, mgallien, mpyne, #frameworks Cc: mpyne, dhaumann, ngraham, kde-frameworks-devel, jtamate, bruns, ashaposhnikov, michaelh, astippich, spoorun, abraham

D12233: Avoid manipulation of lists with quadratic complexity

2018-07-15 Thread Stefan Brüns
bruns marked 4 inline comments as done. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12233 To: bruns, #baloo, michaelh, mgallien, mpyne, #frameworks Cc: mpyne, dhaumann, ngraham, kde-frameworks-devel, jtamate, bruns, ashaposhnikov, michaelh, astippich, spoorun, abraham

D12233: Avoid manipulation of lists with quadratic complexity

2018-07-15 Thread Stefan Brüns
bruns removed a reviewer: Baloo. This revision is now accepted and ready to land. Restricted Application added a subscriber: Baloo. REPOSITORY R293 Baloo BRANCH arcpatch-D12233 REVISION DETAIL https://phabricator.kde.org/D12233 To: bruns, michaelh, mgallien, mpyne, #frameworks Cc: #baloo,

D12233: Avoid manipulation of lists with quadratic complexity

2018-07-15 Thread Stefan Brüns
bruns edited reviewers, added: Baloo; removed: michaelh. REPOSITORY R293 Baloo BRANCH arcpatch-D12233 REVISION DETAIL https://phabricator.kde.org/D12233 To: bruns, mgallien, mpyne, #frameworks, #baloo Cc: #baloo, mpyne, dhaumann, ngraham, kde-frameworks-devel, jtamate, bruns, ashaposhnik

D12233: Avoid manipulation of lists with quadratic complexity

2018-07-15 Thread Stefan Brüns
bruns added a comment. Sorry for the noise - apparently, if one requests changes, adopt the revision, does the changes, the "Changes requested" flag is not cleared and can not be cleared ... REPOSITORY R293 Baloo BRANCH arcpatch-D12233 REVISION DETAIL https://phabricator.kde.org/D122

D12233: Avoid manipulation of lists with quadratic complexity

2018-07-15 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:39944a5e4fa4: Avoid manipulation of lists with quadratic complexity (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12233?vs=35068&id=37835 REVIS

D14237: Make Konqi look good in HiDPI

2018-07-20 Thread Stefan Brüns
bruns added a comment. Try zopflipng ... REPOSITORY R263 KXmlGui REVISION DETAIL https://phabricator.kde.org/D14237 To: ngraham, broulik, cfeck, #frameworks Cc: bruns, kde-frameworks-devel, michaelh, ngraham

D13869: [solid] Notify when interface to mounted fs is lost

2018-07-23 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > anthonyfieroni wrote in udisksmanager.cpp:220 > According to docs, > https://dbus.freedesktop.org/doc/dbus-specification.html#standard-interfaces-objectmanager, > interfaces should not be empty but a dict of looses ones. This does not answer my qu

D13869: [solid] Notify when interface to mounted fs is lost

2018-07-23 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > anthonyfieroni wrote in udisksmanager.cpp:220 > Old code is complete broken, about me, it does not check any interface except > that it's empty. Can you please just answer the question? REPOSITORY R245 Solid REVISION DETAIL https://phabricato

D14308: [Exe Thumbnailer] Improve icon selection algorithm

2018-07-24 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > icoutils_common.cpp:28 > +{ > +// compute difference of areas > +const qreal desiredAspectRatio = (desiredHeight > 0) ? desiredWidth / > static_cast(desiredHeight) : 0; Can not see an area calculation here ... > broulik wrote in icoutils_c

D14237: Make Konqi look good in HiDPI

2018-07-27 Thread Stefan Brüns
bruns added a comment. I have traced the image with GIMPs selection tool, "convert selection to path" and export path as SVG. IMHO works best for tracing pixel art. F6154333: konqui.png F6154332: konqui.svg

D14401: Don't instantiate a QStringRef into a QString only to search in a QStringList

2018-07-27 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > parser.cpp:58 > parseMetadataXml(xml); > -} else if (elements.contains(xml.name().toString())) { > +} else if (stringList_contains_stringRef(elements, xml.name())) { > item = parseXml(xml); Y

D14237: Make Konqi look good in HiDPI

2018-07-28 Thread Stefan Brüns
bruns added a comment. In D14237#299794 , @ngraham wrote: > sodipodi:absref="/home/sbruens/Downloads/aboutkde_doubled2.png" > ... > width="449.999" /> > Just delete the image element from the SVG and

D14308: [Exe Thumbnailer] Improve icon selection algorithm

2018-07-30 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > broulik wrote in icoutils_common.cpp:33 > I'm open to suggestions … also technically you cannot assume the icon is > square Where do you see any assumption about square icons? REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde

D14308: [Exe Thumbnailer] Improve icon selection algorithm

2018-07-30 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > anthonyfieroni wrote in icoutils_common.cpp:33 > It should be qFuzzyIsNull no? @anthonyfieroni Why? REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D14308 To: broulik, #frameworks, dfaure, ngraham, pali, vonreth, antlar

D14308: [Exe Thumbnailer] Improve icon selection algorithm

2018-07-30 Thread Stefan Brüns
bruns added a comment. Criteria for a good selection algorithm: 1. If there is an exact fit, use it 2. If all candidates have the same aspect ratio, use the best fit - prefer slight downscaling over slight upscaling - prefer slight upscaling over large downscaling 3. If aspect

D14308: [Exe Thumbnailer] Improve icon selection algorithm

2018-07-30 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > anthonyfieroni wrote in icoutils_common.cpp:33 > Comparing doubles should be done by epsilon not by <= 0.0 Not in general. There is nothing inexact here. Also, if difference ~= 0, it does not matter if you scale by 1, 2, -1 or -2. REPOSITORY R32

D14308: [Exe Thumbnailer] Improve icon selection algorithm

2018-07-30 Thread Stefan Brüns
bruns added a comment. I think the following yields quite good results: qreal distance(int width, int height, int desiredWidth, int desiredHeight) { auto targetSamples = desiredWidth * desiredHeight; auto xscale = (1.0 * desiredWidth) / width; auto yscale = (1

D13869: [solid] Notify when interface to mounted fs is lost

2018-08-05 Thread Stefan Brüns
bruns requested changes to this revision. bruns added a comment. This revision now requires changes to proceed. The change needs a proper commit message. A reference to the bug tracker is not sufficient. The commit message should (at least) include: - Description of the setup - T

D13869: [solid] Notify when interface to mounted fs is lost

2018-08-05 Thread Stefan Brüns
bruns added a comment. In D13869#303563 , @ngraham wrote: > FWIW, we got a thumbs up from someone in https://bugs.kde.org/show_bug.cgi?id=394348: A thumbs up is nice, but this does not catch the thumbs down of all the persons who are exp

D13869: [solid] Notify when interface to mounted fs is lost

2018-08-05 Thread Stefan Brüns
bruns added a comment. In D13869#303877 , @anthonyfieroni wrote: > About loop device it should present as well > http://storaged.org/doc/udisks2-api/2.6.3/gdbus-org.freedesktop.UDisks2.Loop.html > After all you have ony hypothetical argume

D13869: [solid] Notify when interface to mounted fs is lost

2018-08-05 Thread Stefan Brüns
bruns added a comment. In D13869#303861 , @anthonyfieroni wrote: > Since you make question to new code I make for old one, did you know why Device interfaces is ever used, it's look wrong and why interfaces should be empty, after all my tests,

D14661: Force reevaluation of Predicates if interfaces are removed

2018-08-06 Thread Stefan Brüns
bruns created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY If an application wants to show only specicific devices based on predicate matching, and o

D14661: Force reevaluation of Predicates if interfaces are removed

2018-08-06 Thread Stefan Brüns
bruns edited the test plan for this revision. bruns added reviewers: Frameworks, broulik, ngraham. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D14661 To: bruns, #frameworks, broulik, ngraham Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14661: Force reevaluation of Predicates if interfaces are removed

2018-08-06 Thread Stefan Brüns
bruns added a comment. This is an alternative to D13869 REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D14661 To: bruns, #frameworks, broulik, ngraham Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14707: autotests: skip '/' fstab check with zfs

2018-08-09 Thread Stefan Brüns
bruns added a comment. But doesn't KMountPoint::currentMountPoints() use mtab? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14707 To: dfaure Cc: bruns, kde-frameworks-devel, michaelh, ngraham

D14707: autotests: skip '/' fstab check with zfs

2018-08-09 Thread Stefan Brüns
bruns added a comment. In D14707#305749 , @dfaure wrote: > Yes it does, and that's where I get the "zfs" information from. > > But the test that fails is in the following method, which uses possibleMountPoints() i.e. fstab. Yes, I see

D14308: [Exe Thumbnailer] Improve icon selection algorithm

2018-08-09 Thread Stefan Brüns
bruns added a comment. qreal distance(int width, int height, int desiredWidth, int desiredHeight) { auto targetSamples = desiredWidth * desiredHeight; auto xscale = (1.0 * desiredWidth) / width; auto yscale = (1.0 * desiredHeight) / height; // clamp to

D14733: [KFormat] Add human readble list displaying function

2018-08-10 Thread Stefan Brüns
bruns added a comment. You should start with a) a use case b) show it actually matches the needs of different languages. For e.g. englisch and the most european languages, two different positions are sufficent. Also, the AndListType and the UnitListType are identical. What is missing

D14733: [KFormat] Add human readble list displaying function

2018-08-10 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > kformattest.cpp:396 > + > +QStringList testDate; > +QCOMPARE(format.formatList(testDate), QStringLiteral("")); Why test**Date**? > kformat.h:287 > * > - * e.g. given formatDuration(6), returns "1.0 minutes" > + * e.g. give

D14707: autotests: skip '/' fstab check with zfs

2018-08-10 Thread Stefan Brüns
bruns added a comment. In D14707#305773 , @dfaure wrote: > OK maybe I misunderstood the comment "Welcome to the wondeful world of / on zfs with boot-environments :)." > > If it's not related to zfs then two options: > > 1. allowing for no

D14661: Force reevaluation of Predicates if interfaces are removed

2018-08-12 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > anthonyfieroni wrote in udisksmanager.cpp:239 > Your code seems to work, but i don't get it - why device is added again? How > predicate will work on removed device? Adding it again allows the Predicate to determine if this device is still releva

D14308: [Exe Thumbnailer] Improve icon selection algorithm

2018-08-13 Thread Stefan Brüns
bruns added a comment. Should be "Prefer higher BPP" (bits per pixel), DPI is dots per inch, i.e. spatial resolution. INLINE COMMENTS > icoutils_common.cpp:100 > +// but they store the actual depth of the icon extracted in custom > text: > +// qtbase/src/plugins/imageformats

D14308: [Exe Thumbnailer] Improve icon selection algorithm

2018-08-13 Thread Stefan Brüns
bruns requested changes to this revision. bruns added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > broulik wrote in icoutils_common.cpp:100 > No, that is the actual file name on qtbase git dev https://github.com/qt/qtbase/blob/5.11/src/plugins/imageformats/ic

D14308: [Exe Thumbnailer] Improve icon selection algorithm

2018-08-13 Thread Stefan Brüns
bruns added a comment. In D14308#308137 , @broulik wrote: > > Should be "Prefer higher BPP" > > That's just the internal changelog on Phabricator. Can we ship this already.. See "Summary: " ... INLINE COMMENTS > icoutils_common.cpp:

D14308: [Exe Thumbnailer] Improve icon selection algorithm

2018-08-13 Thread Stefan Brüns
bruns added a comment. Bits ber pixel? REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D14308 To: broulik, #frameworks, dfaure, ngraham, pali, vonreth, antlarr, bruns Cc: anthonyfieroni, bruns

D14661: Force reevaluation of Predicates if interfaces are removed

2018-08-14 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. bruns marked an inline comment as done. Closed by commit R245:8e9fc89918c4: Force reevaluation of Predicates if interfaces are removed (authored by bruns). REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabri

D14237: Make Konqi look good in HiDPI

2018-08-15 Thread Stefan Brüns
bruns added a comment. In D14237#309615 , @cfeck wrote: > In D14237#299303 , @bruns wrote: > > > I have traced the image with GIMPs selection tool, "convert selection to path" and export path as SVG.

D18603: Implement more tags for taglib writer

2019-01-31 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > taglibwritertest.cpp:276 > +QFAIL("This mime type is not supported by the extractor. Likely a > newer KDE Frameworks version is required."); > +KFileMetaData::Extractor* ex = extractorList.first(); > +KFileMetaData::SimpleExtractionR

D18601: Rewrite taglib writer to use property interface

2019-01-31 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > taglibwriter.cpp:123 > > -TagLib::Tag* tags = file.tag(); > +TagLib::File *file = openFile(&stream, mimeType); > +if (!file) { use a smart pointer (unique_ptr, QScopedPtr) here, and get rid of the delete below. REPOSITORY R286 KFil

D18604: Implement support for writing rating information for taglib writer

2019-01-31 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > taglibwritertest.cpp:409 > +QTest::addRow("mp3_0") > +<< QStringLiteral("mp3") > +<< QStringLiteral("audio/mpeg3") What are the differences between the various mp3 test cases? > taglibwriter.cpp:153 > +//map the rating v

D18601: Rewrite taglib writer to use property interface

2019-01-31 Thread Stefan Brüns
bruns added a comment. I think this becomes better structured when you: 1. Create one function per file type, with parameters (Taglib::Filestream, KFM::PropertyMap) 2. From this function, call a generic `updateProperties(oldProperties, newProperties) -> mergedProperties` 3. Call the

D18237: Fix ResultIterator

2019-01-31 Thread Stefan Brüns
bruns added a comment. The correct way to **fix** this is by allocating a new ResultIteratorPrivate and "copying" the QStringList. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D18237 To: aacid, #baloo, bruns, poboiko, apol Cc: apol, kde-frameworks-devel, #baloo, ash

D18237: Fix ResultIterator

2019-01-31 Thread Stefan Brüns
bruns added a comment. Oh, and you forgot to undelete the operator=, so this is still BIC ... REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D18237 To: aacid, #baloo, bruns, poboiko, apol Cc: apol, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoo

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

2019-02-03 Thread Stefan Brüns
bruns requested changes to this revision. This revision now requires changes to proceed. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D18698 To: poboiko, #baloo, #frameworks, ngraham, bruns Cc: bruns, ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astip

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

2019-02-03 Thread Stefan Brüns
bruns added a comment. To which "file1"/"file2" are you referring in the summary? The commit log should be self contained. INLINE COMMENTS > kinotify.cpp:431 > +} > +addWatch(QFile::decodeName(path), d->mode, d->flags); > +} else { `QF

D18664: Baloo engine: treat every non-success code as a failure

2019-02-03 Thread Stefan Brüns
bruns added a comment. In general, these changes seem worthwhile. But as already said, splitting these into two parts - QDebug etc vs. Error handling - is a must. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D18664 To: valeriymalov, #baloo, bruns, poboiko Cc: ngra

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

2019-02-04 Thread Stefan Brüns
bruns requested changes to this revision. bruns added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kinotify.cpp:430 > +// HACK: For some reason, it.fileInfo() returns an > empty QFileInfo for `path` here > +// C

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

2019-02-04 Thread Stefan Brüns
bruns added a comment. In D18698#404820 , @poboiko wrote: > Explained the race condition in summary, expanded test to check if watches were installed correctly. I am not sure if I understood your description correctly, but I am quite sure

D17302: Add test for adding properties to result

2019-02-04 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > resulttest.cpp:63 > +QCOMPARE(keywords->type(), QVariant::List); > +QCOMPARE(keywords->toList().count(), 2); > + This looks wrong to me ... How many items do you get when you append "keyword3" first and ["keyword1", "keyword2"] next

D18665: Cleanup taglib writer test

2019-02-04 Thread Stefan Brüns
bruns accepted this revision. This revision is now accepted and ready to land. REPOSITORY R286 KFileMetaData BRANCH simplify_writing_test REVISION DETAIL https://phabricator.kde.org/D18665 To: astippich, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun,

D18603: Implement more tags for taglib writer

2019-02-04 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > taglibwriter.cpp:134 > TagLib::MPEG::File file(&stream, > TagLib::ID3v2::FrameFactory::instance(), false); > -auto savedProperties = file.properties(); > -writeGenericProperties(savedProperties, properties); I think you hav

D18601: Rewrite taglib writer to use property interface

2019-02-04 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > taglibwriter.cpp:70 > > -if (properties.contains(Property::Title)) { > -title = > QStringToTString(properties.value(Property::Title).toString()); > -tags->setTitle(title); > +void writeGenericProperties(TagLib::PropertyMap &old

D18604: Implement support for writing rating information for taglib writer

2019-02-04 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > taglibwriter.cpp:229 > +writeID3v2Tags(file.ID3v2Tag(), properties); > +} > file.save(); Is this the same as auto id3Tags = dynamic_cast(file.tag()); if (id3Tags) { ... } ? > taglibwriter.cpp:279 > +if (mp

D18638: Avoid using trimmed method

2019-02-04 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > highlighter.cpp:311 > { > -if (text.trimmed().isEmpty() || !d->active || !d->spellCheckerFound) { > +if (!hasNotEmptyText(text) || !d->active || !d->spellCheckerFound) { > return; if (std::all_of(text.cbegin(), text.cend(), [

D16643: Correct the accept flag of the event object on DragMove

2019-02-04 Thread Stefan Brüns
bruns added a comment. As fvogt already said, the current (old) code is wrong, the event should not be blindly ignored (line 97). The second change fvogt mentioned is IMHO handled quite strangely here - `!m_enabled || m_temporaryInhibition` is the inverse of `!m_enabled || m_temporaryIn

D16643: Correct the accept flag of the event object on DragMove

2019-02-05 Thread Stefan Brüns
bruns added a comment. In D16643#405468 , @fvogt wrote: > > The second change fvogt mentioned is IMHO handled quite strangely here - !m_enabled || m_temporaryInhibition is the inverse of !m_enabled || m_temporaryInhibition, so doing a event->ign

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

2019-02-05 Thread Stefan Brüns
bruns added a comment. In D18698#405636 , @poboiko wrote: > Added code to work with first entry that pops from FilteredDirIterator (that is the directory itself) > Test still works; but we should emit `created()` signal for it as well.

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

2019-02-05 Thread Stefan Brüns
bruns added a comment. In D18698#405504 , @poboiko wrote: > > I am not sure if I understood your description correctly, but I am quite sure this race condition does not exist - the files/folders inside the moved folder are not created/moved one

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

2019-02-05 Thread Stefan Brüns
bruns added a comment. Addendum: I have just checked, https://phabricator.kde.org/source/baloo/browse/master/src/file/filewatch.cpp$144 filewatch relies on the code here to emit created events for all children, so we need the traversal code not only for "moved", but also for "created",

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

2019-02-05 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > kinotify.cpp:429 > +if (!it.next().isEmpty()) { > +d->addWatch(it.filePath()); > +} This is broken now, as you fail to emit created for any child. REPOSITORY R293 Baloo REVISION DET

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

2019-02-05 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > bruns wrote in kinotify.cpp:429 > This is broken now, as you fail to emit created for any child. Ignore, looked at the wrong line ... REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D18698 To: poboiko, #baloo, #frameworks, n

D18649: [GridViewKCM] improve contrast and legibility for delegates' inline hover buttons

2019-02-05 Thread Stefan Brüns
bruns added a comment. The inline version looks a little bit more polished, while the other one looks literally "just put on top". The "button" version though somewhat resembles the "selection" buttons/emblems in dolphin. There is a very tiny issue with the button version, although the b

D16643: Correct the accept flag of the event object on DragMove

2019-02-05 Thread Stefan Brüns
bruns added a comment. In D16643#405814 , @fvogt wrote: > @trmdi: Can you do the small change @bruns suggested? ^ Then it can be landed and everyone's happy :-) 👍 REPOSITORY R296 KDeclarative REVISION DETAIL https://phabricator.kde.

D18604: Implement support for writing rating information for taglib writer

2019-02-05 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > astippich wrote in taglibwriter.cpp:229 > No. Mp3 supports ID3v1, ID3v2 or Ape tags, so simply casting to ID3v2 is not > possible. but in case file.tag() returns an ApeTag, the dynamic_cast will return a nullptr. This is a dynamic_cast, not a stat

D18776: Make sure only directories are added to the inotify watcher

2019-02-05 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, Frameworks, poboiko, ngraham. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY Although watchFolder is only called for directories, there

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

2019-02-06 Thread Stefan Brüns
bruns added a comment. In D18698#406755 , @poboiko wrote: > Something like that? I've decided not to emit `created` signal from inside the function, just to have a bit less branching in the code (and documented this behavior, since it might be a

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

2019-02-06 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > kinotify.h:198 > + * and installs watches for all subdirectories (including @param path) > + */ > +void handleDirCreated(const QString& path); Can you add a note created events are only emitted if the file/directory is not exclude by t

D18776: Make sure only directories are added to the inotify watcher

2019-02-06 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:ee7ea5b55de9: Make sure only directories are added to the inotify watcher (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18776?vs=51008&id=51072

D18825: Replace recursive isDirHidden with iterative one, allow const argument

2019-02-07 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, Frameworks, poboiko, ngraham. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY The non-unix implementation modifies its QDir argument to

D18828: [balooctl] Actually abort a malformed command instead of just saying so

2019-02-07 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, Frameworks, poboiko, ngraham. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY A single directory entry can either be included or exclude

D18827: [balooctl] Add missing help for "config set", normalize string

2019-02-07 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, Frameworks, poboiko, ngraham. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY The "contentIndexing" command was not listed for "balooctl

D18829: [balooctl] Normalize include/exclude pathes before using it for the config

2019-02-07 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, Frameworks, poboiko, ngraham. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY A directory can be added multiple times to the config by j

D18829: [balooctl] Normalize include/exclude pathes before using it for the config

2019-02-07 Thread Stefan Brüns
bruns added a dependent revision: D18830: Handle folders matching substrings of included/excluded folders correctly. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D18829 To: bruns, #baloo, #frameworks, poboiko, ngraham Cc: kde-frameworks-devel, ashaposhnikov, michaelh, a

D18830: Handle folders matching substrings of included/excluded folders correctly

2019-02-07 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, Frameworks, poboiko, ngraham. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY FileIndexerConfig::shouldFolderBeIndexe(...) calls folderI

D17245: Add string formatting function to property info

2019-02-07 Thread Stefan Brüns
bruns added a comment. `src/formatstrings.h` should be named `src/formatstrings_p.h` REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D17245 To: astippich, broulik, bruns, mgallien, #frameworks Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, a

D18827: [balooctl] Add missing help for "config set", normalize string

2019-02-07 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:3a31a7ed0e29: [balooctl] Add missing help for "config set", normalize string (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18827?vs=51129&id=511

D18825: Replace recursive isDirHidden with iterative one, allow const argument

2019-02-07 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:38698b1a4c9e: Replace recursive isDirHidden with iterative one, allow const argument (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18825?vs=5112

D18828: [balooctl] Actually abort a malformed command instead of just saying so

2019-02-07 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:a040c71e353d: [balooctl] Actually abort a malformed command instead of just saying so (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18828?vs=511

D18833: Get rid of mostly unused filePathToStat overload

2019-02-07 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, Frameworks, poboiko, ngraham. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY It is only used in one place to retrieve the device ID, wh

D18835: Handle as container element in SVG

2019-02-07 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, Frameworks, astippich, poboiko, ngraham. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY is a container element as , so handle both the

D18851: [Extractor] Exclude GPG encrypted data from being indexed

2019-02-08 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, Frameworks, ngraham, poboiko. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY application/pgp-encrypted may be encoded as base64 and thu

D18851: [Extractor] Exclude GPG encrypted data from being indexed

2019-02-08 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:3aa911d4a0ac: [Extractor] Exclude GPG encrypted data from being indexed (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18851?vs=51183&id=51184 R

D18835: Handle as container element in SVG

2019-02-08 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R286:f86639982645: Handle as container element in SVG (authored by bruns). REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18835?vs=51145&id=51231 REVISION DET

D18833: Get rid of mostly unused filePathToStat overload

2019-02-08 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:ad4d29c4b170: Get rid of mostly unused filePathToStat overload (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18833?vs=51143&id=51232 REVISION D

D18829: [balooctl] Normalize include/exclude pathes before using it for the config

2019-02-08 Thread Stefan Brüns
bruns marked an inline comment as done. bruns added inline comments. INLINE COMMENTS > ngraham wrote in configcommand.cpp:43 > Does this need to be a `while`? Why not a regular old `if`? Are you trying to > handle the case where it ends with multiple trailing slashes? Exactly REPOSITORY R293

D18829: [balooctl] Normalize include/exclude pathes before using it for the config

2019-02-08 Thread Stefan Brüns
bruns marked an inline comment as done. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D18829 To: bruns, #baloo, #frameworks, poboiko, ngraham Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

2019-02-08 Thread Stefan Brüns
bruns requested changes to this revision. This revision now requires changes to proceed. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D18698 To: poboiko, #baloo, #frameworks, ngraham, bruns Cc: bruns, ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astip

D18877: Optimize Baloo::File copy assign operator, fix Baloo::File::load(url)

2019-02-09 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, Frameworks, ngraham, poboiko. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY There is no need to delete and create a new Private on ass

D18873: add baloo engine debugging category

2019-02-09 Thread Stefan Brüns
bruns requested changes to this revision. bruns added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > baloo.categories:4 > kf5.kio.kio_timeline Baloo Timeline (KIO) DEFAULT_SEVERITY [WARNING] > IDENTIFIER [Baloo::KIO_TIMELINE] > +org.kde.baloo.engine Baloo engi

D18873: add baloo engine debugging category

2019-02-09 Thread Stefan Brüns
bruns added a comment. In D18873#408662 , @mlaurent wrote: > DEFAULT_SEVERITY [WARNING] is false too as you don't force it. i.e. add "DEFAULT_SEVERITY Warning" in ecm_qt_declare_logging_category compare with https://phabricator.kde.org/

D18664: Baloo engine: treat every non-success code as a failure

2019-02-09 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > documentdatadb.cpp:42 > +if (rc) { > +qCDebug(ENGINE) << "DocumentDataDB::create" << mdb_strerror(rc); > +return 0; Warning > documentdatadb.cpp:54 > +if (rc) { > +qCDebug(ENGINE) << "DocumentDataDB::open" << mdb_str

D18664: Baloo engine: treat every non-success code as a failure

2019-02-09 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > valeriymalov wrote in documentdatadb.cpp:107 > Trying to delete a non-existent entry seems like an error to me Deleting is an idempotent operation, so e.g. doing it twice is fine. There may be multiple events queued leading to the deletion of a dat

D18873: add baloo engine debugging category

2019-02-09 Thread Stefan Brüns
bruns added a comment. Looks good to me. @mlaurent ? REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D18873 To: valeriymalov, #baloo, mlaurent, bruns Cc: bruns, mlaurent, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, abrahams

D17245: Add string formatting function to property info

2019-02-09 Thread Stefan Brüns
bruns accepted this revision. This revision is now accepted and ready to land. REPOSITORY R286 KFileMetaData BRANCH display_value REVISION DETAIL https://phabricator.kde.org/D17245 To: astippich, broulik, bruns, mgallien, #frameworks Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhniko

<    3   4   5   6   7   8   9   10   11   12   >