D7337: Port rest of scripting API to QJSValue-based solution
dhaumann closed this revision. dhaumann added a comment. Committed: https://cgit.kde.org/ktexteditor.git/commit/?id=fc510adaecf1dc83416eaf4392925ee4f3c5a1e0 REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D7337 To: dhaumann, carewolf, cullmann Cc: kwrite-devel, #frameworks
D7337: Port rest of scripting API to QJSValue-based solution
cullmann accepted this revision. cullmann added a comment. Looks ok for me. To shorten the code, I would have used const auto cursor = cursorFromScriptValue(jscursor); at most places, but that is taste, the explicit type makes more clear what the type is ;=) REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D7337 To: dhaumann, carewolf, cullmann Cc: kwrite-devel, #frameworks
D7337: Port rest of scripting API to QJSValue-based solution
dhaumann updated this revision to Diff 18307. dhaumann added a comment. Remove KTextEditor::Cursor and KTextEditor::Range based API. make test still succeeds. REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7337?vs=18210=18307 REVISION DETAIL https://phabricator.kde.org/D7337 AFFECTED FILES src/script/katescriptdocument.cpp src/script/katescriptdocument.h src/script/katescriptview.cpp src/script/katescriptview.h To: dhaumann, carewolf, cullmann Cc: kwrite-devel, #frameworks
D7337: Port rest of scripting API to QJSValue-based solution
cullmann added a comment. Think that would be nice. Then we always have only that tuple of functions, makes it less confusing (and less to maintain). REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D7337 To: dhaumann, carewolf, cullmann Cc: kwrite-devel, #frameworks
D7337: Port rest of scripting API to QJSValue-based solution
dhaumann added a comment. Yes, Allen is correct: Essentially, we could remove all helper functions that use KTE::Range/Cursor. Shall I update the patch? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D7337 To: dhaumann, carewolf, cullmann Cc: kwrite-devel, #frameworks
D7337: Port rest of scripting API to QJSValue-based solution
carewolf added a comment. A few of the cursor/range functions are used, but not all of them. You could probably rewrite the places using them to use line+col instead. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D7337 To: dhaumann, carewolf, cullmann Cc: kwrite-devel, #frameworks
D7337: Port rest of scripting API to QJSValue-based solution
cullmann added a comment. Question: Do we need the overloads with the KTextEditor::Cursor at all or should we limit that to line/column + QJSValue? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D7337 To: dhaumann, carewolf, cullmann Cc: kwrite-devel, #frameworks
D7337: Port rest of scripting API to QJSValue-based solution
carewolf accepted this revision. carewolf added a comment. This revision is now accepted and ready to land. Looks good to me REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D7337 To: dhaumann, carewolf, cullmann Cc: kwrite-devel, #frameworks
D7337: Port rest of scripting API to QJSValue-based solution
dhaumann created this revision. Restricted Application added subscribers: Frameworks, kwrite-devel. Restricted Application added a project: Frameworks. REVISION SUMMARY This completes the scripting API for QJSValue-based Cursor and Range functions. The changes are rather simple, sometimes code is just moved around to make the header file more readable. TEST PLAN make test succeeds. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D7337 AFFECTED FILES src/script/katescriptdocument.cpp src/script/katescriptdocument.h src/script/katescriptview.cpp src/script/katescriptview.h To: dhaumann, carewolf, cullmann Cc: kwrite-devel, #frameworks