----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125262/#review85502 -----------------------------------------------------------
KF5CoreAddonsMacros.cmake (line 3) <https://git.reviewboard.kde.org/r/125262/#comment59097> typo: SERIVCE -> SERVICE Does this break SC, if DEFAULT_SERVICE_TYPE must be specified? It sounds like it should be the default value instead, so that existing cmake code can keep working. src/lib/plugin/desktopfileparser.h (line 39) <https://git.reviewboard.kde.org/r/125262/#comment59098> typo: Definiton -> Definition src/lib/plugin/desktopfileparser.h (line 45) <https://git.reviewboard.kde.org/r/125262/#comment59099> defintion? ;) src/lib/plugin/desktopfileparser.h (line 47) <https://git.reviewboard.kde.org/r/125262/#comment59100> hm? it *is* marked as const... src/lib/plugin/desktopfileparser.cpp (line 272) <https://git.reviewboard.kde.org/r/125262/#comment59105> should be isEmpty, not just the more specific isNull? src/lib/plugin/desktopfileparser.cpp (line 318) <https://git.reviewboard.kde.org/r/125262/#comment59104> defs.reserve(paths.count()) src/lib/plugin/kpluginmetadata.h (line 169) <https://git.reviewboard.kde.org/r/125262/#comment59101> give -> given src/lib/plugin/kpluginmetadata.h (line 173) <https://git.reviewboard.kde.org/r/125262/#comment59102> s/is intended// src/lib/plugin/kpluginmetadata.cpp (line 106) <https://git.reviewboard.kde.org/r/125262/#comment59103> inconsistent placement of '&', should after the space; best solution is to run astyle-kdelibs over the code. - David Faure On Sept. 16, 2015, 4:47 p.m., Alex Richardson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125262/ > ----------------------------------------------------------- > > (Updated Sept. 16, 2015, 4:47 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kcoreaddons > > > Description > ------- > > KPluginMetadata("foo.desktop") or kcoreaddons_desktop_to_json() do not always > result in the correct JSON for example when a property is > supposed to be a list. the .desktop file parser in kcoreaddons so far had no > way of know this so instead of treating the entry as a > comma-separated list it would simply return a JSON string. > > This is a follow up to https://git.reviewboard.kde.org/r/121672/ with a > different solution that also handles service types that are not compiled into > the kcoreaddons library but instead parses the installed servicetype .desktop > file. > > It also contains a bit of code-deduplication and porting to categorized > logging which is not directly related to this patch but it would be some > effort to split that into a separate RR. > > It is a series of commits with the following messages: > > > desktopparser: avoid unnecessary utf8 decoding > > Parse ServiceType files when reading .desktop files > > > Remove lots of duplicated code for desktop{tojson,fileparser}.cpp > > The only reason these were copy-pasted are minor differences in the > output which is now fixed by using qInstallMessageHandler and the > ability to generate JSON compatible with the first published version > of desktoptojson (which is hopefully no longer used and can be removed > soon) > > QCommandLineParser uses -v for --version so just use --verbose > > Otherwise the whole QCommandlineOption is ignored and there is no way > to enable verbose mode > > desktopparser: Use more categorized logging > > > desktopparser: Allow passing relative paths to service type files > > > Add KPluginMetaData::fromDesktopFile() > > This function allows specifying a list of service type files to be > parsed when loading the .desktop file. > > desktopparser: Improve warning messages and add new unit test > > The new test checks how the desktop parser handles service type files > with invalid property definitions > > desktopparser: Fix parsing of double and bool values > > QString::compare returns 0 on equal and make sure that we don't assign > the parsed double to an integer local variable > > Add another unit test for desktop parsing with service types > > Test that all supported types are converted correctly > > Allow setting service types in kcoreaddons_desktop_to_json() > > > Diffs > ----- > > KF5CoreAddonsMacros.cmake acfcaa3069991395d83923bcc30cd08f231c30eb > autotests/data/servicetypes/bad-groups-input.desktop PRE-CREATION > autotests/data/servicetypes/bad-groups-servicetype.desktop PRE-CREATION > autotests/data/servicetypes/example-input.desktop PRE-CREATION > autotests/data/servicetypes/example-servicetype.desktop PRE-CREATION > autotests/data/servicetypes/fake-kdevelopplugin.desktop PRE-CREATION > autotests/desktoptojsontest.cpp 64373d5be930426dd8a1f8e455e33c411a4795fd > autotests/kpluginmetadatatest.cpp 3af5e1b842b0bc231a5ac001112e141f751d2ff5 > src/desktoptojson/CMakeLists.txt 94a199d8fa44a21b15e24c2e4f42551adada8f72 > src/desktoptojson/desktoptojson.h bfa38b0f5ddd0581ad176d854614bc9c80dd139a > src/desktoptojson/desktoptojson.cpp > 82626b256df6b3bd106e6d4c6fad84d7d970af37 > src/desktoptojson/main.cpp 9bac8ff55d005d1944c04f557aa9601de2b0ca15 > src/lib/plugin/desktopfileparser.h 98d47ddf0f877c4a25928026b3d5fe169cfc9e75 > src/lib/plugin/desktopfileparser.cpp > 0b03eb154deb58840c91c12658780c0d492b593c > src/lib/plugin/kpluginmetadata.h 183b0d0583259f7ed74e97858a68c5c388fd0a9a > src/lib/plugin/kpluginmetadata.cpp b13d6dd52827cc03d9473600aa4d2bab8a95a1d4 > > Diff: https://git.reviewboard.kde.org/r/125262/diff/ > > > Testing > ------- > > Added some unit test and they pass > > > Thanks, > > Alex Richardson > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
