[education/rkward/devel/workspace_output] /: Also convert the 'plain generic requests' to generic return value handling

Thomas Friedrichsmeier null at kde.org
Sat Oct 24 21:46:54 BST 2020


Git commit d04906e2cf4c77b9e6535c1f0503a1528d1f384e by Thomas Friedrichsmeier.
Committed on 24/10/2020 at 20:46.
Pushed by tfry into branch 'devel/workspace_output'.

Also convert the 'plain generic requests' to generic return value handling

M  +1    -0    ChangeLog
M  +13   -12   rkward/rbackend/rkrbackend.cpp
M  +3    -3    rkward/rbackend/rkrbackend.h
M  +15   -16   rkward/rbackend/rkrinterface.cpp
M  +3    -3    rkward/rbackend/rkrinterface.h
M  +2    -4    rkward/rbackend/rpackages/rkward/R/base_overrides.R
M  +1    -1    rkward/rbackend/rpackages/rkward/R/rk.KDE_GUI-functions.R

https://invent.kde.org/education/rkward/commit/d04906e2cf4c77b9e6535c1f0503a1528d1f384e

diff --git a/ChangeLog b/ChangeLog
index 5f84804b..294e3487 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,4 @@
+- Fixed: Calling (rk.)select.list() without a title would fail
 - Hide or remove several purely internal functions (most can still be assessed from the rkward namespace as rkward:::xyz())
 
 --- Version 0.7.2 - UNRELEASED
diff --git a/rkward/rbackend/rkrbackend.cpp b/rkward/rbackend/rkrbackend.cpp
index ea106df6..acb90c07 100644
--- a/rkward/rbackend/rkrbackend.cpp
+++ b/rkward/rbackend/rkrbackend.cpp
@@ -2,7 +2,7 @@
                           rkrbackend  -  description
                              -------------------
     begin                : Sun Jul 25 2004
-    copyright            : (C) 2004 - 2019 by Thomas Friedrichsmeier
+    copyright            : (C) 2004 - 2020 by Thomas Friedrichsmeier
     email                : thomas.friedrichsmeier at kdemail.net
  ***************************************************************************/
 
@@ -947,7 +947,7 @@ SEXP doSubstackCall (SEXP call) {
 
 
 	auto ret = RKRBackend::this_pointer->handleHistoricalSubstackRequest(list);
-	if (!ret.warning.isEmpty()) Rf_warning(RKRBackend::fromUtf8(ret.warning));  // print warnings, first, as errors are - an error
+	if (!ret.warning.isEmpty()) Rf_warning(RKRBackend::fromUtf8(ret.warning));  // print warnings, first, as errors will cause a stop
 	if (!ret.error.isEmpty()) Rf_error(RKRBackend::fromUtf8(ret.error));
 
 	return RKRSupport::QVariantToSEXP(ret.ret);
@@ -958,8 +958,11 @@ SEXP doPlainGenericRequest (SEXP call, SEXP synchronous) {
 
 	R_CheckUserInterrupt ();
 
-	QStringList ret = RKRBackend::this_pointer->handlePlainGenericRequest (RKRSupport::SEXPToStringList (call), RKRSupport::SEXPToInt (synchronous));
-	return RKRSupport::StringListToSEXP (ret);
+	auto ret = RKRBackend::this_pointer->handlePlainGenericRequest(RKRSupport::SEXPToStringList(call), RKRSupport::SEXPToInt(synchronous));
+	if (!ret.warning.isEmpty()) Rf_warning(RKRBackend::fromUtf8(ret.warning));  // print warnings, first, as errors will cause a stop
+	if (!ret.error.isEmpty()) Rf_error(RKRBackend::fromUtf8(ret.error));
+
+	return RKRSupport::QVariantToSEXP(ret.ret);
 }
 
 // Function to handle several simple calls from R code, that do not need any special arguments, or interaction with the frontend process.
@@ -1492,7 +1495,7 @@ void RKRBackend::printCommand (const QString &command) {
 
 	QStringList params ("highlightRCode");
 	params.append (command);
-	QString highlighted = handlePlainGenericRequest (params, true).value (0);
+	QString highlighted = handlePlainGenericRequest(params, true).ret.toString();
 	catToOutputFile (highlighted);
 }
 
@@ -1639,7 +1642,7 @@ QString getLibLoc() {
 	return RKRBackendProtocolBackend::dataDir () + "/.rkward_packages/" + QString::number (RKRBackend::this_pointer->r_version / 10);
 }
 
-QStringList RKRBackend::handlePlainGenericRequest (const QStringList &parameters, bool synchronous) {
+GenericRRequestResult RKRBackend::handlePlainGenericRequest (const QStringList &parameters, bool synchronous) {
 	RK_TRACE (RBACKEND);
 
 	RBackendRequest request (synchronous, RBackendRequest::PlainGenericRequest);
@@ -1653,19 +1656,17 @@ QStringList RKRBackend::handlePlainGenericRequest (const QStringList &parameters
 		output_file = parameters.value (1);
 		if (parameters.length () > 2) {
 			RK_ASSERT (parameters.value (2) == "SILENT");
-			return QStringList ();		// For automated testing and previews. The frontend should not be notified, here
+			return GenericRRequestResult();  // For automated testing and previews. The frontend should not be notified, here
 		}
 		request.params["call"] = parameters;
 	} else if (parameters.value(0) == "home") {
-		QStringList ret;
-		if (parameters.value(1) == "home") ret << RKRBackendProtocolBackend::dataDir();
-		else if (parameters.value(1) == "lib") ret << getLibLoc();
-		return ret;
+		if (parameters.value(1) == "home") return GenericRRequestResult(RKRBackendProtocolBackend::dataDir());
+		else if (parameters.value(1) == "lib") return GenericRRequestResult(getLibLoc());
 	} else {
 		request.params["call"] = parameters;
 	}
 	handleRequest (&request);
-	return request.params.value ("return").toStringList ();
+	return request.getResult();
 }
 
 void RKRBackend::initialize (const QString &locale_dir) {
diff --git a/rkward/rbackend/rkrbackend.h b/rkward/rbackend/rkrbackend.h
index f6c879c4..3fedfe62 100644
--- a/rkward/rbackend/rkrbackend.h
+++ b/rkward/rbackend/rkrbackend.h
@@ -107,10 +107,10 @@ public:
 	void handleRequest (RBackendRequest *request) { handleRequest (request, true); };
 /** A relic of history. In contrast to handlePlainGenericRequest(), these requests support running sub-commands. However, the remaining requests which are currently handled this way
 should probably be converted to dedicated RKRBackendRequest's in the future. See also handlePlainGenericRequest(). */
-	GenericRRequestResult handleHistoricalSubstackRequest (const QStringList &list);
-/** Sends a request to the frontend and returns the result (an empty QStringList in case of asynchronous requests). Note that this function has considerable overlap with
+	GenericRRequestResult handleHistoricalSubstackRequest(const QStringList &list);
+/** Sends a request to the frontend and returns the result (empty in case of asynchronous requests). Note that this function has considerable overlap with
 handleHistoricalSubstackRequest(). Exactly which requests get handled by which function is somewhat arbitrary, ATM. However, request that do not need sub-commands to be run, should generally be converted to use handlePlainGenericRequest(). (And probably all historicalSubstackRequests should be replaced!) */
-	QStringList handlePlainGenericRequest (const QStringList &parameters, bool synchronous);
+	GenericRRequestResult handlePlainGenericRequest(const QStringList &parameters, bool synchronous);
 	RCommandProxy* fetchNextCommand ();
 
 /** The command currently being executed. */
diff --git a/rkward/rbackend/rkrinterface.cpp b/rkward/rbackend/rkrinterface.cpp
index de5c3aaa..f617ae3b 100644
--- a/rkward/rbackend/rkrinterface.cpp
+++ b/rkward/rbackend/rkrinterface.cpp
@@ -405,7 +405,7 @@ void RInterface::handleRequest (RBackendRequest* request) {
 		command_requests.append (request);
 		processHistoricalSubstackRequest (request->params["call"].toStringList (), parent, request);
 	} else if (request->type == RBackendRequest::PlainGenericRequest) {
-		request->setResult(QVariant(processPlainGenericRequest(request->params["call"].toStringList())));
+		request->setResult(processPlainGenericRequest(request->params["call"].toStringList()));
 		RKRBackendProtocolFrontend::setRequestCompleted (request);
 	} else if (request->type == RBackendRequest::Started) {
 		// The backend thread has finished basic initialization, but we still have more to do...
@@ -551,8 +551,7 @@ void RInterface::pauseProcessing (bool pause) {
 	else locked -= locked & User;
 }
 
-#warning Convert this to GenericRRequestResult
-QStringList RInterface::processPlainGenericRequest (const QStringList &calllist) {
+GenericRRequestResult RInterface::processPlainGenericRequest(const QStringList &calllist) {
 	RK_TRACE (RBACKEND);
 
 	QString call = calllist.value (0);
@@ -564,17 +563,17 @@ QStringList RInterface::processPlainGenericRequest (const QStringList &calllist)
 		QDir::setCurrent (calllist.value (1));
 		emit (backendWorkdirChanged());
 	} else if (call == "highlightRCode") {
-		return (QStringList (RKCommandHighlighter::commandToHTML (calllist.value (1))));
+		return GenericRRequestResult(RKCommandHighlighter::commandToHTML(calllist.value(1)));
 	} else if (call == "quit") {
 		RKWardMainWindow::getMain ()->close ();
 		// if we're still alive, quitting was cancelled
-		return (QStringList ("FALSE"));
+		return GenericRRequestResult::makeError(i18n("Quitting was cancelled"));
 	} else if (call == "preLocaleChange") {
 		int res = KMessageBox::warningContinueCancel (0, i18n ("A command in the R backend is trying to change the character encoding. While RKWard offers support for this, and will try to adjust to the new locale, this operation may cause subtle bugs, if data windows are currently open. Also the feature is not well tested, yet, and it may be advisable to save your workspace before proceeding.\nIf you have any data editor opened, or in any doubt, it is recommended to close those first (this will probably be auto-detected in later versions of RKWard). In this case, please choose 'Cancel' now, then close the data windows, save, and retry."), i18n ("Locale change"));
-		if (res != KMessageBox::Continue) return (QStringList ("FALSE"));
+		if (res != KMessageBox::Continue) return GenericRRequestResult::makeError(i18n("Changing the locale was cancelled by user"));
 	} else if (call == "listPlugins") {
 		RK_ASSERT (calllist.count () == 1);
-		return RKComponentMap::getMap ()->listPlugins ();
+		return GenericRRequestResult(RKComponentMap::getMap()->listPlugins());
 	} else if (call == "setPluginStatus") {
 		QStringList params = calllist.mid (1);
 		RK_ASSERT ((params.size () % 3) == 0);
@@ -601,16 +600,16 @@ QStringList RInterface::processPlainGenericRequest (const QStringList &calllist)
 
 		QStringList results = RKSelectListDialog::doSelect (QApplication::activeWindow(), title, choices, preselects, multiple);
 		if (results.isEmpty ()) results.append ("");	// R wants to have it that way
-		return (results);
+		return GenericRRequestResult(results);
 	} else if (call == "commandHistory") {
 		if (calllist.value (1) == "get") {
-			return (RKConsole::mainConsole ()->commandHistory ());
+			return GenericRRequestResult(RKConsole::mainConsole()->commandHistory());
 		} else {
 			RKConsole::mainConsole ()->setCommandHistory (calllist.mid (2), calllist.value (1) == "append");
 		}
 	} else if (call == "getWorkspaceUrl") {
 		QUrl url = RKWorkplace::mainWorkplace ()->workspaceURL ();
-		if (!url.isEmpty ()) return (QStringList (url.url ()));
+		if (!url.isEmpty()) return GenericRRequestResult(url.url());
 	} else if (call == "workplace.layout") {
 		if (calllist.value (1) == "set") {
 			if (calllist.value (2) == "close") RKWorkplace::mainWorkplace ()->closeAll ();
@@ -618,7 +617,7 @@ QStringList RInterface::processPlainGenericRequest (const QStringList &calllist)
 			RKWorkplace::mainWorkplace ()->restoreWorkplace (list);
 		} else {
 			RK_ASSERT (calllist.value (1) == "get");
-			return (RKWorkplace::mainWorkplace ()->makeWorkplaceDescription ());
+			return GenericRRequestResult(RKWorkplace::mainWorkplace ()->makeWorkplaceDescription ());
 		}
 	} else if (call == "set.window.placement.hint") {
 		RKWorkplace::mainWorkplace ()->setWindowPlacementOverrides (calllist.value (1), calllist.value (2), calllist.value (3));
@@ -632,7 +631,7 @@ QStringList RInterface::processPlainGenericRequest (const QStringList &calllist)
 		lines.append (calllist.value (1));
 		lines.append (QString ());
 		lines.append ("R version (compile time): " + calllist.value (2));
-		return (lines);
+		return GenericRRequestResult(lines);
 	} else if (call == "recordCommands") {
 		RK_ASSERT (calllist.count () == 3);
 		QString filename = calllist.value (1);
@@ -643,7 +642,7 @@ QStringList RInterface::processPlainGenericRequest (const QStringList &calllist)
 			command_logfile.close ();
 		} else {
 			if (command_logfile_mode != NotRecordingCommands) {
-				return (QStringList ("Attempt to start recording, while already recording commands. Ignoring.)"));
+				return GenericRRequestResult(QVariant(), i18n("Attempt to start recording, while already recording commands. Ignoring.)"));
 			} else {
 				command_logfile.setFileName (filename);
 				bool ok = command_logfile.open (QIODevice::WriteOnly | QIODevice::Truncate);
@@ -651,7 +650,7 @@ QStringList RInterface::processPlainGenericRequest (const QStringList &calllist)
 					if (unfiltered) command_logfile_mode = RecordingCommandsUnfiltered;
 					else command_logfile_mode = RecordingCommands;
 				} else {
-					return (QStringList ("Could not open file for writing. Not recording commands"));
+					return GenericRRequestResult::makeError(i18n("Could not open file for writing. Not recording commands"));
 				}
 			}
 		}
@@ -662,11 +661,11 @@ QStringList RInterface::processPlainGenericRequest (const QStringList &calllist)
 	} else if (call == "switchLanguage") {
 		RKMessageCatalog::switchLanguage (calllist.value (1));
 	} else {
-		return (QStringList ("Error: unrecognized request '" + call + "'."));
+		return GenericRRequestResult::makeError(i18n("Error: unrecognized request '%1'", call));
 	}
 
 	// for those calls which were recognized, but do not return anything
-	return QStringList ();
+	return GenericRRequestResult();
 }
 
 void RInterface::processHistoricalSubstackRequest (const QStringList &calllist, RCommand *parent_command, RBackendRequest *request) {
diff --git a/rkward/rbackend/rkrinterface.h b/rkward/rbackend/rkrinterface.h
index 52f3415b..85e7d98c 100644
--- a/rkward/rbackend/rkrinterface.h
+++ b/rkward/rbackend/rkrinterface.h
@@ -2,7 +2,7 @@
                           rkrinterface.h  -  description
                              -------------------
     begin                : Fri Nov 1 2002
-    copyright            : (C) 2002 - 2018 by Thomas Friedrichsmeier
+    copyright            : (C) 2002 - 2020 by Thomas Friedrichsmeier
     email                : thomas.friedrichsmeier at kdemail.net
  ***************************************************************************/
 
@@ -101,8 +101,8 @@ private:
 
 /** helper function to handle backend requests that (may) involve running additional R-"sub"-commands. TODO; This should probably be merged with processRBackendRequest.*/
 	void processHistoricalSubstackRequest (const QStringList &calllist, RCommand *parent_command, RBackendRequest *request);
-/** helper function to handle the bulk backend of requests that do not involve running sub-commands, and need a return value that can easily be represented in a QStringList() */
-	QStringList processPlainGenericRequest (const QStringList &calllist);
+/** helper function to handle the bulk backend of requests that do not involve running sub-commands */
+	GenericRRequestResult processPlainGenericRequest (const QStringList &calllist);
 /** helper function to handle backend requests that do not inolve running sub-commands. */
 	void processRBackendRequest (RBackendRequest *request);
 
diff --git a/rkward/rbackend/rpackages/rkward/R/base_overrides.R b/rkward/rbackend/rpackages/rkward/R/base_overrides.R
index 5918fa93..4760b67f 100644
--- a/rkward/rbackend/rpackages/rkward/R/base_overrides.R
+++ b/rkward/rbackend/rpackages/rkward/R/base_overrides.R
@@ -36,8 +36,7 @@
 "q" <- function (save = "default", status = 0, runLast = TRUE, ...) {
 	# test if this is running in RKWard, otherwise pass through to the actual q()
 	if (isTRUE(.rk.inside.rkward.session())){
-		res <- .rk.do.plain.call ("quit")
-		if (length (res) && (res == "FALSE")) stop ("Quitting was cancelled")
+		.rk.do.plain.call ("quit")
 	} else {
 		base:::q(save = save, status = status, runLast = runLast)
 	}
@@ -53,8 +52,7 @@
 #' @export
 "Sys.setlocale" <- function (category = "LC_ALL", locale = "", ...) {
 	if (category == "LC_ALL" || category == "LC_CTYPE" || category == "LANG") {
-		allow <- .rk.do.plain.call ("preLocaleChange", NULL)
-		if (length (allow) && (allow == "FALSE")) stop ("Changing the locale was cancelled by user")
+		 .rk.do.plain.call ("preLocaleChange", NULL)
 
 		ret <- base::Sys.setlocale (category, locale, ...)
 
diff --git a/rkward/rbackend/rpackages/rkward/R/rk.KDE_GUI-functions.R b/rkward/rbackend/rpackages/rkward/R/rk.KDE_GUI-functions.R
index 617dbfab..d3c4e66f 100755
--- a/rkward/rbackend/rpackages/rkward/R/rk.KDE_GUI-functions.R
+++ b/rkward/rbackend/rpackages/rkward/R/rk.KDE_GUI-functions.R
@@ -164,7 +164,7 @@
 	params <- list ()
 
 	# serialize all parameters
-	params[1] <- as.character (title)
+	params[1] <- if(is.null(title)) "" else as.character (title)
 	if (multiple) params[2] <- "multi"
 	else params[2] <- "single"
 	params[3] <- as.character (preselect.len)




More information about the rkward-tracker mailing list