D9247: Extend Scripting API to allow executing commands
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R39:72c51e15659f: Extend Scripting API to allow executing commands (authored by dhaumann, committed by cullmann). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9247?vs=39575=39755 REVISION DETAIL https://phabricator.kde.org/D9247 AFFECTED FILES src/script/katescriptview.cpp src/script/katescriptview.h To: dhaumann, cullmann, mwolff, kfunk Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars, dhaumann
D9247: Extend Scripting API to allow executing commands
cullmann accepted this revision. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D9247 To: dhaumann, cullmann, mwolff, kfunk Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars, dhaumann
D9247: Extend Scripting API to allow executing commands
dhaumann marked 2 inline comments as done. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D9247 To: dhaumann, cullmann, mwolff, kfunk Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars, dhaumann
D9247: Extend Scripting API to allow executing commands
dhaumann updated this revision to Diff 39575. dhaumann added a comment. Restricted Application edited subscribers, added: kde-frameworks-devel, kwrite-devel; removed: Frameworks. - Apply proposed fixes - Merge branch 'master' into CommandScript REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9247?vs=23628=39575 BRANCH CommandScript (branched from master) REVISION DETAIL https://phabricator.kde.org/D9247 AFFECTED FILES src/script/katescriptview.cpp src/script/katescriptview.h To: dhaumann, cullmann, mwolff, kfunk Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars, dhaumann, #frameworks
D9247: Extend Scripting API to allow executing commands
mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.
in general this sounds like a good idea. Also adding getters to the API would
be good.
there are two typos in your commit message, but stupid phabricator does not
let me review that:
`Rante::invalid`
`seletcion`
INLINE COMMENTS
> katescriptview.cpp:124
> +
> +QJSValue KateScriptView::executeCommand(const QString & command,
> +const QString & args,
here and below: wrong placement of &
> katescriptview.cpp:135
> +ok = false;
> +message = QStringLiteral("Command not found: ") + command;
> +} else {
i18n, prevent string puzzle
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D9247
To: dhaumann, cullmann, mwolff, kfunk
Cc: #frameworks, michaelh, kevinapavew, ngraham, demsking, cullmann, sars,
dhaumann
D9247: Extend Scripting API to allow executing commands
cullmann added a comment. > Well, the scripting commands are also available on the command line as commands, aren't they? So you could essentially call yourself. Yeah, that is true, but that is no difference to you are calling yourself indirectly in javascript itself. Perhaps a bit more hidden. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D9247 To: dhaumann, cullmann, mwolff, kfunk Cc: #frameworks, kevinapavew, ngraham, demsking, cullmann, sars, dhaumann
D9247: Extend Scripting API to allow executing commands
dhaumann added a comment. Well, the scripting commands are also available on the command line as commands, aren't they? So you could essentially call yourself. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D9247 To: dhaumann, cullmann, mwolff, kfunk Cc: #frameworks, kevinapavew, ngraham, demsking, cullmann, sars, dhaumann
D9247: Extend Scripting API to allow executing commands
cullmann added a comment. How can we have infinite recursion? By calling a command that triggers again the script function we did call the command in? If that is the only problem, I see no issue, that can happen with normal function calls already, perhaps we could guard chained command calls inside executeCommand, thought (aka guard that not twice the same "command" string arrives via set) REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D9247 To: dhaumann, cullmann, mwolff, kfunk Cc: #frameworks, kevinapavew, ngraham, demsking, cullmann, sars, dhaumann
D9247: Extend Scripting API to allow executing commands
dhaumann retitled this revision from "Extend Scripting API to allow executong commands" to "Extend Scripting API to allow executing commands". REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D9247 To: dhaumann, cullmann, mwolff, kfunk Cc: #frameworks, kevinapavew, ngraham, demsking, cullmann, sars, dhaumann
D9247: Extend Scripting API to allow executing commands
dhaumann edited the summary of this revision. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D9247 To: dhaumann, cullmann, mwolff, kfunk Cc: #frameworks, kevinapavew, ngraham, demsking, cullmann, sars, dhaumann
