This revision was automatically updated to reflect the committed changes.
Closed by commit R249:9040bff75a5d: Port ki18n from QtScript to QtQml (authored
by carewolf).
REPOSITORY
R249 KI18n
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D7033?vs=39526=39629
REVISION DETAIL
dhaumann accepted this revision.
dhaumann added a comment.
This revision is now accepted and ready to land.
I guess it's better to accept this now than later to have a longer testing
period at Akademy and until the frameworks release.
REPOSITORY
R249 KI18n
BRANCH
master
REVISION DETAIL
dfaure added a comment.
+1, looks good to me (but I'm no QJSEngine expert)
REPOSITORY
R249 KI18n
REVISION DETAIL
https://phabricator.kde.org/D7033
To: carewolf, ilic
Cc: dfaure, kde-frameworks-devel, huftis, ngraham, dhaumann, ltoscano, kfunk,
michaelh, bruns
carewolf added a comment.
I solved the Ts.acall. Unfortunately I forgot how arc works, so it became a
new entry https://phabricator.kde.org/D14769
REVISION DETAIL
https://phabricator.kde.org/D7033
To: carewolf, ilic
Cc: kde-frameworks-devel, huftis, ngraham, dhaumann, ltoscano, kfunk,
carewolf updated this revision to Diff 39526.
carewolf added a comment.
Fixed acall using a pure javascript bridge
REPOSITORY
R249 KI18n
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D7033?vs=39460=39526
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D7033
carewolf added a comment.
Note the latest patch still has a maximum number of arguments for acall. I
checked all the usages, and the only place that uses acall with a variable set
of arguments is the sr (Serbian) translation. Since supporting a variable
number of arguments would require
carewolf updated this revision to Diff 39460.
carewolf added a comment.
Updated and ensured autotests passes.
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D7033?vs=17489=39460
REVISION DETAIL
https://phabricator.kde.org/D7033
AFFECTED FILES
CMakeLists.txt
Restricted Application edited subscribers, added: kde-frameworks-devel;
removed: Frameworks.
REPOSITORY
R249 KI18n
REVISION DETAIL
https://phabricator.kde.org/D7033
To: carewolf, ilic
Cc: kde-frameworks-devel, huftis, ngraham, dhaumann, ltoscano, kfunk, michaelh,
bruns, #frameworks
carewolf added a comment.
No, but I will see if I can find some. Otherwise feel free to take over an
fix the remaining issues.
REPOSITORY
R249 KI18n
REVISION DETAIL
https://phabricator.kde.org/D7033
To: carewolf, ilic
Cc: ngraham, dhaumann, ltoscano, kfunk, #frameworks
ngraham added a comment.
@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. :)
REPOSITORY
R249 KI18n
REVISION DETAIL
https://phabricator.kde.org/D7033
To: carewolf, ilic
Cc: ngraham, dhaumann, ltoscano, kfunk,
ilic added a comment.
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.
REPOSITORY
R249 KI18n
REVISION DETAIL
kfunk added a comment.
Could someone fluent in QtScript/QtQML review this so we can merge it?
INLINE COMMENTS
> ktranscript.cpp:96
> // Interface functions.
> -Q_INVOKABLE QScriptValue load(); // actually has variable length
> argument list
> -Q_INVOKABLE QScriptValue
dhaumann added a comment.
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.
REPOSITORY
R249 KI18n
REVISION DETAIL
https://phabricator.kde.org/D7033
To: carewolf, ilic
ltoscano added a comment.
@ilic, the repository was set incorrectly but it's really for KI18n.
REPOSITORY
R249 KI18n
REVISION DETAIL
https://phabricator.kde.org/D7033
To: carewolf, ilic
Cc: ltoscano, kfunk, #frameworks
ltoscano changed the repository for this revision from R252 Framework
Integration to R249 KI18n.
REPOSITORY
R249 KI18n
REVISION DETAIL
https://phabricator.kde.org/D7033
To: carewolf, ilic
Cc: kfunk, #frameworks
kfunk added a comment.
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.
ltoscano added a reviewer: ilic.
REPOSITORY
R252 Framework Integration
REVISION DETAIL
https://phabricator.kde.org/D7033
To: carewolf, ilic
Cc: #frameworks
17 matches
Mail list logo