Review Request 125262: Parse service type files when loading from .desktop

[Alex Richardson]

> On Sept. 16, 2015, 6:14 p.m., David Faure wrote:
> > KF5CoreAddonsMacros.cmake, line 3
> > <https://git.reviewboard.kde.org/r/125262/diff/1/?file=404292#file404292line3>
> >
> >     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.

It uses DEFAULT_SERVICE_TYPE if neither is set, message(DEPRECATION) will only cause an error if CMAKE_ERROR_DEPRECATED is set.

>From the docs: 
DEPRECATION = CMake Deprecation Error or Warning if variable CMAKE_ERROR_DEPRECATED or CMAKE_WARN_DEPRECATED is enabled, respectively, else no message.


> On Sept. 16, 2015, 6:14 p.m., David Faure wrote:
> > src/lib/plugin/desktopfileparser.h, line 47
> > <https://git.reviewboard.kde.org/r/125262/diff/1/?file=404304#file404304line47>
> >
> >     hm? it *is* marked as const...

Forgot to remove this comment when I changed the way I stored data.


- Alex


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125262/#review85502
-----------------------------------------------------------


On Sept. 17, 2015, 12:37 a.m., Alex Richardson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125262/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2015, 12:37 a.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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150916/c9fde1ff/attachment.html>