Port ki18n from QtScript to QtQml
Needs ReviewPublic

Authored by carewolf on Aug 1 2017, 12:56 PM.



Moves away from the deprecated QtScript module

Diff Detail

R252 Framework Integration
Lint Skipped
Unit Tests Skipped
carewolf created this revision.Aug 1 2017, 12:56 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptAug 1 2017, 12:56 PM
Restricted Application added a subscriber: Frameworks. · View Herald Transcript
kfunk added a subscriber: kfunk.Aug 28 2017, 8:37 AM

Just wanted to add this: Really great work! -- I wanted to raise this issue earlier on the KF5 mailing list: ki18n was the last major framework that depended on the deprecated QtScript. Nice to see it being ported! <3

Can't really review this code either, though. I don't now enough about the QtScript/QtQML differences.

ltoscano changed the repository for this revision from R252 Framework Integration to R249 KI18n.Aug 28 2017, 7:46 PM
ltoscano added a subscriber: ltoscano.

@ilic, the repository was set incorrectly but it's really for KI18n.

The changes to from QScriptValue to QJSValue look quite good (similar to the ones done in KTextEditor). I cannot really comment on the FIXMEs you have added, though.

kfunk added a comment.Sep 6 2017, 7:49 AM

Could someone fluent in QtScript/QtQML review this so we can merge it?


Consistency: Use const QString &

ilic edited edge metadata.Sep 6 2017, 7:59 AM

I can't say about QtScript to QtQML conversion, but wrt. those FIXMEs, removing variable argument lists obviously breaks compatibility, and especially the acall function makes no sense without it.

@carewolf, have you had a chance to work on this? It would be a shame for such a lovely patch to fall by the wayside. :)

No, but I will see if I can find some. Otherwise feel free to take over an fix the remaining issues.

huftis added a subscriber: huftis.Fri, Jun 15, 5:41 PM
Restricted Application edited subscribers, added: kde-frameworks-devel; removed: Frameworks. · View Herald TranscriptFri, Jun 15, 5:41 PM