Port ki18n from QtScript to QtQml
Needs ReviewPublic

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

Details

Reviewers
ilic
Summary

Moves away from the deprecated QtScript module

Diff Detail

Repository
R252 Framework Integration
Lint
Lint Skipped
Unit
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.Mon, Aug 28, 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.Mon, Aug 28, 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.Wed, Sep 6, 7:49 AM

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

src/ktranscript.cpp
96

Consistency: Use const QString &

ilic added a comment.Wed, Sep 6, 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.