Review Request 121672: Properly convert .desktop files that have an associated servicetype

Alex Richardson arichardson.kde at gmail.com
Fri Jan 9 14:57:31 UTC 2015



> On Jan. 8, 2015, 3:26 p.m., Milian Wolff wrote:
> > While better than before, I still don't like it very much. As soon as another $type-property is added, we'd need to patch this here. Imo, it just shows that desktop2json should be deprecated and people should start writing .json files directly.

I fully agree, desktoptojson is only a tool to help the porting and should go away. This patch is only a hack to get more files to convert correctly. Unfortunately I am very busy at the moment and will only be able to post a better version in 1-2 weeks.


> On Jan. 8, 2015, 3:26 p.m., Milian Wolff wrote:
> > src/lib/plugin/desktopfileparser.cpp, line 361
> > <https://git.reviewboard.kde.org/r/121672/diff/1/?file=335788#file335788line361>
> >
> >     why not listProperties().contains(), similar below - i.e. why the (non const!) temporaries above?

Oops, they should definitively be const. Calling listProperties().contains(), would cause the set to be constructed again.


- Alex


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


On Dec. 26, 2014, 5:21 p.m., Alex Richardson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121672/
> -----------------------------------------------------------
> 
> (Updated Dec. 26, 2014, 5:21 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> -------
> 
> This ensures that properties that are defined to be of type QStringList
> or int or bool are properly converted to the right JSON type.
> 
> Not sure if this code should also be part of KF5CoreAddons.so, since it
> does increase the library size quite a bit. It would however be very
> useful for kcoreaddons_desktop_to_json(), so that the initial conversion
> to JSON does not have to be done by hand.
> 
> I probably don't have all the service types that exist installed on my
> system so I might be missing some properties. I included the script to
> generate the list of these properties, so that missing properties can
> be added by anyone who has the required servicetypes/*.desktop files
> installed.
> 
> 
> Diffs
> -----
> 
>   src/lib/plugin/read-servicetypes.py PRE-CREATION 
>   autotests/desktoptojsontest.cpp 64373d5be930426dd8a1f8e455e33c411a4795fd 
>   src/lib/plugin/desktopfileparser.cpp b1b5440b48e4fd412932a7d7e794d641b1406699 
> 
> Diff: https://git.reviewboard.kde.org/r/121672/diff/
> 
> 
> Testing
> -------
> 
> Unit test works
> 
> 
> Thanks,
> 
> Alex Richardson
> 
>

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


More information about the Kde-frameworks-devel mailing list