Re: Review Request 121122: Allow loading KPluginMetaData from a .desktop file

2014-12-01 Thread Sebastian Kügler 

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

(Updated Dec. 1, 2014, 2:57 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Plasma, Alexander Richardson, David Faure, 
and Marco Martin.


Repository: kcoreaddons


Description
---

This set of patches moves the code to decode and json-serialize from 
desktoptojson into its own class, used by the KPluginMetaData(QString file) 
constructor.

These functions are not exported, desktoptojson directly includes this class. 
I've done it this way to avoid littering the API, yet removing as much 
duplicated code from desktoptojson as possible. These functions have gone into 
their own class, and made static, since they don't carry state, anyway.

The purpose of this patch is to allow the package indexer in kpackage (the 
split-out framework containing Plasma::Package) to create a correct, 
KPlugin-compatible cache file from package metadata. This patch allows us to 
just feed a .desktop file into KPluginMetaData and get back correct 
deserialized KPluginMetaData objects. It allows us to cut out the KService 
dependency in KPackage.

In particular, this is split up into the following patches (you can find it in 
git kcoreaddons[sebas/kpluginmetadata] as well):


Remove now-duplicated code in desktoptojson

Use escapeValue, deserializeList and convertToJson from
desktopfileparser.

Move the bits to parse .desktop files into its own class

This encapsulates the desktop file parseing logic into its own class.
It's fairly self-contained, all static code. This also makes it easier
to share this code with the desktoptojson binary.

Allow to read KPluginMetaData from .desktop files

Move the logic to create a KPluginMetaData object from desktoptojson
into KPluginMetaData.

This allows to load metadata from a .desktop file, making it possible to
load desktop files without going through KPluginInfo (and thus
KService). This is useful for conversion to json data, which will be
used to index packages in the KPackage framework for faster lookups,
similar to the index proof-of-concept for KPluginTrader.

Add an autotest for loading KPluginMetaData from a .desktop file


Diffs
-

  autotests/data/fakeplugin.desktop PRE-CREATION 
  autotests/kpluginmetadatatest.cpp a1aaf80c8a5e25c2ae031a80fd1f8f96fa5924a0 
  src/desktoptojson/CMakeLists.txt c2d2b8780ab6cb34686bc7a43a3b6d4eaeb12b1e 
  src/desktoptojson/desktoptojson.cpp 3f0cfb129730cf7ba8adbf9c331d967f8a04d4e8 
  src/lib/CMakeLists.txt 1dc56270ab5157af706b7745cfa88ae11e16184a 
  src/lib/plugin/desktopfileparser.h PRE-CREATION 
  src/lib/plugin/desktopfileparser.cpp PRE-CREATION 
  src/lib/plugin/kpluginmetadata.h 67b68a50225e8ac82706caa849fb330567cee1d2 
  src/lib/plugin/kpluginmetadata.cpp f3e68c927370ad9212e8dfc07677ec17c52fdbdd 

Diff: https://git.reviewboard.kde.org/r/121122/diff/


Testing
---

Added an autotest which compares a KPluginMetaData object created from a 
desktop file. Also used this code from kpackage.


Thanks,

Sebastian Kügler

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 121122: Allow loading KPluginMetaData from a .desktop file

2014-12-01 Thread Marco Martin 

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

Ship it!


Ship It!

- Marco Martin


On Nov. 14, 2014, 6:08 p.m., Sebastian Kügler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121122/
> ---
> 
> (Updated Nov. 14, 2014, 6:08 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma, Alexander Richardson, David Faure, 
> and Marco Martin.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> This set of patches moves the code to decode and json-serialize from 
> desktoptojson into its own class, used by the KPluginMetaData(QString file) 
> constructor.
> 
> These functions are not exported, desktoptojson directly includes this class. 
> I've done it this way to avoid littering the API, yet removing as much 
> duplicated code from desktoptojson as possible. These functions have gone 
> into their own class, and made static, since they don't carry state, anyway.
> 
> The purpose of this patch is to allow the package indexer in kpackage (the 
> split-out framework containing Plasma::Package) to create a correct, 
> KPlugin-compatible cache file from package metadata. This patch allows us to 
> just feed a .desktop file into KPluginMetaData and get back correct 
> deserialized KPluginMetaData objects. It allows us to cut out the KService 
> dependency in KPackage.
> 
> In particular, this is split up into the following patches (you can find it 
> in git kcoreaddons[sebas/kpluginmetadata] as well):
> 
> 
> Remove now-duplicated code in desktoptojson
> 
> Use escapeValue, deserializeList and convertToJson from
> desktopfileparser.
> 
> Move the bits to parse .desktop files into its own class
> 
> This encapsulates the desktop file parseing logic into its own class.
> It's fairly self-contained, all static code. This also makes it easier
> to share this code with the desktoptojson binary.
> 
> Allow to read KPluginMetaData from .desktop files
> 
> Move the logic to create a KPluginMetaData object from desktoptojson
> into KPluginMetaData.
> 
> This allows to load metadata from a .desktop file, making it possible to
> load desktop files without going through KPluginInfo (and thus
> KService). This is useful for conversion to json data, which will be
> used to index packages in the KPackage framework for faster lookups,
> similar to the index proof-of-concept for KPluginTrader.
> 
> Add an autotest for loading KPluginMetaData from a .desktop file
> 
> 
> Diffs
> -
> 
>   autotests/data/fakeplugin.desktop PRE-CREATION 
>   autotests/kpluginmetadatatest.cpp a1aaf80c8a5e25c2ae031a80fd1f8f96fa5924a0 
>   src/desktoptojson/CMakeLists.txt c2d2b8780ab6cb34686bc7a43a3b6d4eaeb12b1e 
>   src/desktoptojson/desktoptojson.cpp 
> 3f0cfb129730cf7ba8adbf9c331d967f8a04d4e8 
>   src/lib/CMakeLists.txt 1dc56270ab5157af706b7745cfa88ae11e16184a 
>   src/lib/plugin/desktopfileparser.h PRE-CREATION 
>   src/lib/plugin/desktopfileparser.cpp PRE-CREATION 
>   src/lib/plugin/kpluginmetadata.h 67b68a50225e8ac82706caa849fb330567cee1d2 
>   src/lib/plugin/kpluginmetadata.cpp f3e68c927370ad9212e8dfc07677ec17c52fdbdd 
> 
> Diff: https://git.reviewboard.kde.org/r/121122/diff/
> 
> 
> Testing
> ---
> 
> Added an autotest which compares a KPluginMetaData object created from a 
> desktop file. Also used this code from kpackage.
> 
> 
> Thanks,
> 
> Sebastian Kügler
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 121122: Allow loading KPluginMetaData from a .desktop file

2014-12-01 Thread Alexander Richardson 

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

Ship it!


Looks good to me once the open issues have be solved.

- Alexander Richardson


On Nov. 14, 2014, 6:08 nachm., Sebastian Kügler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121122/
> ---
> 
> (Updated Nov. 14, 2014, 6:08 nachm.)
> 
> 
> Review request for KDE Frameworks, Plasma, Alexander Richardson, David Faure, 
> and Marco Martin.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> This set of patches moves the code to decode and json-serialize from 
> desktoptojson into its own class, used by the KPluginMetaData(QString file) 
> constructor.
> 
> These functions are not exported, desktoptojson directly includes this class. 
> I've done it this way to avoid littering the API, yet removing as much 
> duplicated code from desktoptojson as possible. These functions have gone 
> into their own class, and made static, since they don't carry state, anyway.
> 
> The purpose of this patch is to allow the package indexer in kpackage (the 
> split-out framework containing Plasma::Package) to create a correct, 
> KPlugin-compatible cache file from package metadata. This patch allows us to 
> just feed a .desktop file into KPluginMetaData and get back correct 
> deserialized KPluginMetaData objects. It allows us to cut out the KService 
> dependency in KPackage.
> 
> In particular, this is split up into the following patches (you can find it 
> in git kcoreaddons[sebas/kpluginmetadata] as well):
> 
> 
> Remove now-duplicated code in desktoptojson
> 
> Use escapeValue, deserializeList and convertToJson from
> desktopfileparser.
> 
> Move the bits to parse .desktop files into its own class
> 
> This encapsulates the desktop file parseing logic into its own class.
> It's fairly self-contained, all static code. This also makes it easier
> to share this code with the desktoptojson binary.
> 
> Allow to read KPluginMetaData from .desktop files
> 
> Move the logic to create a KPluginMetaData object from desktoptojson
> into KPluginMetaData.
> 
> This allows to load metadata from a .desktop file, making it possible to
> load desktop files without going through KPluginInfo (and thus
> KService). This is useful for conversion to json data, which will be
> used to index packages in the KPackage framework for faster lookups,
> similar to the index proof-of-concept for KPluginTrader.
> 
> Add an autotest for loading KPluginMetaData from a .desktop file
> 
> 
> Diffs
> -
> 
>   autotests/data/fakeplugin.desktop PRE-CREATION 
>   autotests/kpluginmetadatatest.cpp a1aaf80c8a5e25c2ae031a80fd1f8f96fa5924a0 
>   src/desktoptojson/CMakeLists.txt c2d2b8780ab6cb34686bc7a43a3b6d4eaeb12b1e 
>   src/desktoptojson/desktoptojson.cpp 
> 3f0cfb129730cf7ba8adbf9c331d967f8a04d4e8 
>   src/lib/CMakeLists.txt 1dc56270ab5157af706b7745cfa88ae11e16184a 
>   src/lib/plugin/desktopfileparser.h PRE-CREATION 
>   src/lib/plugin/desktopfileparser.cpp PRE-CREATION 
>   src/lib/plugin/kpluginmetadata.h 67b68a50225e8ac82706caa849fb330567cee1d2 
>   src/lib/plugin/kpluginmetadata.cpp f3e68c927370ad9212e8dfc07677ec17c52fdbdd 
> 
> Diff: https://git.reviewboard.kde.org/r/121122/diff/
> 
> 
> Testing
> ---
> 
> Added an autotest which compares a KPluginMetaData object created from a 
> desktop file. Also used this code from kpackage.
> 
> 
> Thanks,
> 
> Sebastian Kügler
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 121122: Allow loading KPluginMetaData from a .desktop file

2014-11-15 Thread Martin Gräßlin 

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



src/lib/plugin/desktopfileparser.h


if all methods are static: why a class instead of a namespace?



src/lib/plugin/kpluginmetadata.h


nitpick: trailing whitespace



src/lib/plugin/kpluginmetadata.h


might be that I'm just blind: I don't find the implementation for these two 
added methods.


- Martin Gräßlin


On Nov. 14, 2014, 7:08 p.m., Sebastian Kügler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121122/
> ---
> 
> (Updated Nov. 14, 2014, 7:08 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma, Alexander Richardson, David Faure, 
> and Marco Martin.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> This set of patches moves the code to decode and json-serialize from 
> desktoptojson into its own class, used by the KPluginMetaData(QString file) 
> constructor.
> 
> These functions are not exported, desktoptojson directly includes this class. 
> I've done it this way to avoid littering the API, yet removing as much 
> duplicated code from desktoptojson as possible. These functions have gone 
> into their own class, and made static, since they don't carry state, anyway.
> 
> The purpose of this patch is to allow the package indexer in kpackage (the 
> split-out framework containing Plasma::Package) to create a correct, 
> KPlugin-compatible cache file from package metadata. This patch allows us to 
> just feed a .desktop file into KPluginMetaData and get back correct 
> deserialized KPluginMetaData objects. It allows us to cut out the KService 
> dependency in KPackage.
> 
> In particular, this is split up into the following patches (you can find it 
> in git kcoreaddons[sebas/kpluginmetadata] as well):
> 
> 
> Remove now-duplicated code in desktoptojson
> 
> Use escapeValue, deserializeList and convertToJson from
> desktopfileparser.
> 
> Move the bits to parse .desktop files into its own class
> 
> This encapsulates the desktop file parseing logic into its own class.
> It's fairly self-contained, all static code. This also makes it easier
> to share this code with the desktoptojson binary.
> 
> Allow to read KPluginMetaData from .desktop files
> 
> Move the logic to create a KPluginMetaData object from desktoptojson
> into KPluginMetaData.
> 
> This allows to load metadata from a .desktop file, making it possible to
> load desktop files without going through KPluginInfo (and thus
> KService). This is useful for conversion to json data, which will be
> used to index packages in the KPackage framework for faster lookups,
> similar to the index proof-of-concept for KPluginTrader.
> 
> Add an autotest for loading KPluginMetaData from a .desktop file
> 
> 
> Diffs
> -
> 
>   autotests/data/fakeplugin.desktop PRE-CREATION 
>   autotests/kpluginmetadatatest.cpp a1aaf80c8a5e25c2ae031a80fd1f8f96fa5924a0 
>   src/desktoptojson/CMakeLists.txt c2d2b8780ab6cb34686bc7a43a3b6d4eaeb12b1e 
>   src/desktoptojson/desktoptojson.cpp 
> 3f0cfb129730cf7ba8adbf9c331d967f8a04d4e8 
>   src/lib/CMakeLists.txt 1dc56270ab5157af706b7745cfa88ae11e16184a 
>   src/lib/plugin/desktopfileparser.h PRE-CREATION 
>   src/lib/plugin/desktopfileparser.cpp PRE-CREATION 
>   src/lib/plugin/kpluginmetadata.h 67b68a50225e8ac82706caa849fb330567cee1d2 
>   src/lib/plugin/kpluginmetadata.cpp f3e68c927370ad9212e8dfc07677ec17c52fdbdd 
> 
> Diff: https://git.reviewboard.kde.org/r/121122/diff/
> 
> 
> Testing
> ---
> 
> Added an autotest which compares a KPluginMetaData object created from a 
> desktop file. Also used this code from kpackage.
> 
> 
> Thanks,
> 
> Sebastian Kügler
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 121122: Allow loading KPluginMetaData from a .desktop file

2014-11-14 Thread Kai Uwe Broulik 

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



src/lib/plugin/desktopfileparser.h


Doesn't seem to be used given it's only passed as reference


- Kai Uwe Broulik


On Nov. 14, 2014, 6:08 nachm., Sebastian Kügler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121122/
> ---
> 
> (Updated Nov. 14, 2014, 6:08 nachm.)
> 
> 
> Review request for KDE Frameworks, Plasma, Alexander Richardson, David Faure, 
> and Marco Martin.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> This set of patches moves the code to decode and json-serialize from 
> desktoptojson into its own class, used by the KPluginMetaData(QString file) 
> constructor.
> 
> These functions are not exported, desktoptojson directly includes this class. 
> I've done it this way to avoid littering the API, yet removing as much 
> duplicated code from desktoptojson as possible. These functions have gone 
> into their own class, and made static, since they don't carry state, anyway.
> 
> The purpose of this patch is to allow the package indexer in kpackage (the 
> split-out framework containing Plasma::Package) to create a correct, 
> KPlugin-compatible cache file from package metadata. This patch allows us to 
> just feed a .desktop file into KPluginMetaData and get back correct 
> deserialized KPluginMetaData objects. It allows us to cut out the KService 
> dependency in KPackage.
> 
> In particular, this is split up into the following patches (you can find it 
> in git kcoreaddons[sebas/kpluginmetadata] as well):
> 
> 
> Remove now-duplicated code in desktoptojson
> 
> Use escapeValue, deserializeList and convertToJson from
> desktopfileparser.
> 
> Move the bits to parse .desktop files into its own class
> 
> This encapsulates the desktop file parseing logic into its own class.
> It's fairly self-contained, all static code. This also makes it easier
> to share this code with the desktoptojson binary.
> 
> Allow to read KPluginMetaData from .desktop files
> 
> Move the logic to create a KPluginMetaData object from desktoptojson
> into KPluginMetaData.
> 
> This allows to load metadata from a .desktop file, making it possible to
> load desktop files without going through KPluginInfo (and thus
> KService). This is useful for conversion to json data, which will be
> used to index packages in the KPackage framework for faster lookups,
> similar to the index proof-of-concept for KPluginTrader.
> 
> Add an autotest for loading KPluginMetaData from a .desktop file
> 
> 
> Diffs
> -
> 
>   autotests/data/fakeplugin.desktop PRE-CREATION 
>   autotests/kpluginmetadatatest.cpp a1aaf80c8a5e25c2ae031a80fd1f8f96fa5924a0 
>   src/desktoptojson/CMakeLists.txt c2d2b8780ab6cb34686bc7a43a3b6d4eaeb12b1e 
>   src/desktoptojson/desktoptojson.cpp 
> 3f0cfb129730cf7ba8adbf9c331d967f8a04d4e8 
>   src/lib/CMakeLists.txt 1dc56270ab5157af706b7745cfa88ae11e16184a 
>   src/lib/plugin/desktopfileparser.h PRE-CREATION 
>   src/lib/plugin/desktopfileparser.cpp PRE-CREATION 
>   src/lib/plugin/kpluginmetadata.h 67b68a50225e8ac82706caa849fb330567cee1d2 
>   src/lib/plugin/kpluginmetadata.cpp f3e68c927370ad9212e8dfc07677ec17c52fdbdd 
> 
> Diff: https://git.reviewboard.kde.org/r/121122/diff/
> 
> 
> Testing
> ---
> 
> Added an autotest which compares a KPluginMetaData object created from a 
> desktop file. Also used this code from kpackage.
> 
> 
> Thanks,
> 
> Sebastian Kügler
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 121122: Allow loading KPluginMetaData from a .desktop file

2014-11-14 Thread Sebastian Kügler 

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

(Updated Nov. 14, 2014, 6:08 p.m.)


Review request for KDE Frameworks, Plasma, Alexander Richardson, David Faure, 
and Marco Martin.


Changes
---

Described testing


Repository: kcoreaddons


Description
---

This set of patches moves the code to decode and json-serialize from 
desktoptojson into its own class, used by the KPluginMetaData(QString file) 
constructor.

These functions are not exported, desktoptojson directly includes this class. 
I've done it this way to avoid littering the API, yet removing as much 
duplicated code from desktoptojson as possible. These functions have gone into 
their own class, and made static, since they don't carry state, anyway.

The purpose of this patch is to allow the package indexer in kpackage (the 
split-out framework containing Plasma::Package) to create a correct, 
KPlugin-compatible cache file from package metadata. This patch allows us to 
just feed a .desktop file into KPluginMetaData and get back correct 
deserialized KPluginMetaData objects. It allows us to cut out the KService 
dependency in KPackage.

In particular, this is split up into the following patches (you can find it in 
git kcoreaddons[sebas/kpluginmetadata] as well):


Remove now-duplicated code in desktoptojson

Use escapeValue, deserializeList and convertToJson from
desktopfileparser.

Move the bits to parse .desktop files into its own class

This encapsulates the desktop file parseing logic into its own class.
It's fairly self-contained, all static code. This also makes it easier
to share this code with the desktoptojson binary.

Allow to read KPluginMetaData from .desktop files

Move the logic to create a KPluginMetaData object from desktoptojson
into KPluginMetaData.

This allows to load metadata from a .desktop file, making it possible to
load desktop files without going through KPluginInfo (and thus
KService). This is useful for conversion to json data, which will be
used to index packages in the KPackage framework for faster lookups,
similar to the index proof-of-concept for KPluginTrader.

Add an autotest for loading KPluginMetaData from a .desktop file


Diffs
-

  autotests/data/fakeplugin.desktop PRE-CREATION 
  autotests/kpluginmetadatatest.cpp a1aaf80c8a5e25c2ae031a80fd1f8f96fa5924a0 
  src/desktoptojson/CMakeLists.txt c2d2b8780ab6cb34686bc7a43a3b6d4eaeb12b1e 
  src/desktoptojson/desktoptojson.cpp 3f0cfb129730cf7ba8adbf9c331d967f8a04d4e8 
  src/lib/CMakeLists.txt 1dc56270ab5157af706b7745cfa88ae11e16184a 
  src/lib/plugin/desktopfileparser.h PRE-CREATION 
  src/lib/plugin/desktopfileparser.cpp PRE-CREATION 
  src/lib/plugin/kpluginmetadata.h 67b68a50225e8ac82706caa849fb330567cee1d2 
  src/lib/plugin/kpluginmetadata.cpp f3e68c927370ad9212e8dfc07677ec17c52fdbdd 

Diff: https://git.reviewboard.kde.org/r/121122/diff/


Testing (updated)
---

Added an autotest which compares a KPluginMetaData object created from a 
desktop file. Also used this code from kpackage.


Thanks,

Sebastian Kügler

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel