qmenumodel

Merge lp://staging/~3v1n0/qmenumodel/converter-improvements into lp://staging/qmenumodel

  1. converter-improvements
  2. Merge into trunk
Proposed by Marco Trevisan (Treviño)
Status: Merged
Approved by: Lukáš Tinkl
Approved revision: 150
Merged at revision: 125
Proposed branch: lp://staging/~3v1n0/qmenumodel/converter-improvements
Merge into: lp://staging/qmenumodel
Prerequisite: lp://staging/~3v1n0/qmenumodel/variant-string-parser
Diff against target: 527 lines (+202/-91)
3 files modified
CMakeLists.txt (+2/-1)
libqmenumodel/src/converter.cpp (+105/-81)
tests/client/convertertest.cpp (+95/-9)
To merge this branch: bzr merge lp://staging/~3v1n0/qmenumodel/converter-improvements
Reviewer Review Type Date Requested Status
Marco Trevisan (Treviño) Abstain
Lukáš Tinkl (community) Approve
Review via email: mp+309273@code.staging.launchpad.net

Commit message

converter: cleanup and add support for more variant types

To post a comment you must log in.
Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

Are there specific use cases for the addition of the newly supported types?

Revision history for this message
Lukáš Tinkl (lukas-kde) wrote :

Suggest using this code simplification: https://pastebin.kde.org/pox8en1rh

Otherwise it looks good

Revision history for this message
Lukáš Tinkl (lukas-kde) wrote :

There's a TODO:

    /* TODO: implement convertions to others types
     * G_VARIANT_TYPE_HANDLE
     * G_VARIANT_TYPE_OBJECT_PATH
     * G_VARIANT_TYPE_SIGNATURE
     * G_VARIANT_TYPE_ANY
     * G_VARIANT_TYPE_BASIC
     * G_VARIANT_TYPE_MAYBE
     * G_VARIANT_TYPE_UNIT
     * G_VARIANT_TYPE_DICT_ENTRY
     * G_VARIANT_TYPE_DICTIONARY
     * G_VARIANT_TYPE_OBJECT_PATH_ARRAY
     */

Needs cleanup imo

Plus one fix comment inline

review: Needs Fixing
lp://staging/~3v1n0/qmenumodel/converter-improvements updated
145. By Lukáš Tinkl

converter: use qUtf8Printable function

146. By Marco Trevisan (Treviño)

Merged variant-string-parser into converter-improvements.

147. By Marco Trevisan (Treviño)

converter: free the strv container

148. By Marco Trevisan (Treviño)

converter: free schema type on destruction

Revision history for this message
Lukáš Tinkl (lukas-kde) wrote :

Yup, works fine, tests passing

review: Approve
lp://staging/~3v1n0/qmenumodel/converter-improvements updated
149. By Marco Trevisan (Treviño)

converter: verify integer type conversions

150. By Marco Trevisan (Treviño)

converter: return a variant when the schema is a variant

Revision history for this message
Lukáš Tinkl (lukas-kde) wrote :

Still good, yup

review: Approve
lp://staging/~3v1n0/qmenumodel/converter-improvements updated
151. By Marco Trevisan (Treviño)

converterTest: add conversion to GVariant and back to verify content is correct

152. By Marco Trevisan (Treviño)

converter: properly set the QByteArray using ctor, or it will be created twice

153. By Marco Trevisan (Treviño)

converterTest: add content conversions back and further from gvariant

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

> Are there specific use cases for the addition of the newly supported types?

No, it's just that there might be such cases.. So I've improved the status also for existing types that were converted using more generic (and expensive) ways.

review: Needs Fixing
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) :
review: Abstain

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches