Add GUI editor for R backend
ClosedPublic

Authored by sirgienko on May 6 2018, 3:09 PM.

Details

Summary

Before, if we have called fix function and our EDITOR was console editor, we have had problems: editor starts as child process and we never can end it. This commit fix it, by building cantor script editor as executable file and setting the editor as editor for R.

BUG: 339127
FIXED-IN: 18.04.1

Test Plan
  1. Open terminal, set console editor as EDITOR (export EDITOR=/bin/nano, for example) and start cantor in this terminal session.
  2. Start R backend, create array (a <- c(1,2,3), for example)
  3. Call fix(a) and check, that you have the problem
  4. Apply the patch
  5. Do 1)-2) again and check, that you start edit a in kate

Diff Detail

Repository
R55 Cantor
Branch
fix-bug-339127
Lint
No Linters Available
Unit
No Unit Test Coverage
sirgienko created this revision.May 6 2018, 3:09 PM
Restricted Application added a project: KDE Edu. · View Herald TranscriptMay 6 2018, 3:09 PM
Restricted Application added a subscriber: KDE Edu. · View Herald Transcript
sirgienko requested review of this revision.May 6 2018, 3:09 PM
sirgienko edited the test plan for this revision. (Show Details)
asemke added a comment.May 6 2018, 5:26 PM

Hard-coding to 'kate' is not a good idea. It's better to find a way to determine the default application for "*.txt" on the used linux desktop or on Windows or on Mac OS X.
QDesktopServices::openUrl() opens the file with the default application. Maybe we can check Qt's source code here to see how the default application is determined or used an already available function in Qt or in KF5.

Hard-coding to 'kate' is not a good idea. It's better to find a way to determine the default application for "*.txt" on the used linux desktop or on Windows or on Mac OS X.
QDesktopServices::openUrl() opens the file with the default application. Maybe we can check Qt's source code here to see how the default application is determined or used an already available function in Qt or in KF5.

But if default application for txt is vim? Or nano? Or another console editor? So, maybe better add editor option to R backend setting?

asemke added a comment.May 6 2018, 5:46 PM

Hard-coding to 'kate' is not a good idea. It's better to find a way to determine the default application for "*.txt" on the used linux desktop or on Windows or on Mac OS X.
QDesktopServices::openUrl() opens the file with the default application. Maybe we can check Qt's source code here to see how the default application is determined or used an already available function in Qt or in KF5.

But if default application for txt is vim? Or nano? Or another console editor? So, maybe better add editor option to R backend setting?

Let's use Cantor's ScriptEditorWidget which is based on KTextEdit that is available together with KF5. This editor is already used in CantorPart::showScriptEditor(). We can extend this widget/editor later if need and use it for everything script related. For BUG 339127 we'll need to compile this script editor as a stand-alone executable and use it in runCommand(). Let's use maybe cantor_scripteditor as the name for this new executable similar to cantor_rserver, etc.

Let's use Cantor's ScriptEditorWidget which is based on KTextEdit that is available together with KF5. This editor is already used in CantorPart::showScriptEditor(). We can extend this widget/editor later if need and use it for everything script related. For BUG 339127 we'll need to compile this script editor as a stand-alone executable and use it in runCommand(). Let's use maybe cantor_scripteditor as the name for this new executable similar to cantor_rserver, etc.

OK, i will try to do it soon.

sirgienko updated this revision to Diff 33834.May 8 2018, 5:03 PM

Replace kate editor by cantor scripteditor

sirgienko edited the summary of this revision. (Show Details)May 8 2018, 5:47 PM
asemke accepted this revision.May 8 2018, 8:20 PM

Very nice! Please add a license header to scripteditor/src/main.cpp and push the change.

This revision is now accepted and ready to land.May 8 2018, 8:20 PM
sirgienko updated this revision to Diff 33841.May 8 2018, 8:27 PM

Add licence header

This revision was automatically updated to reflect the committed changes.