[education/rkward] rkward/rbackend: Use a single means for handling sub-command loops, only

Thomas Friedrichsmeier null at kde.org
Sun Sep 14 19:26:16 BST 2025


Git commit 6b3494a7dd2e54163fe2f4d908d0a9f670efd748 by Thomas Friedrichsmeier.
Committed on 13/09/2025 at 21:04.
Pushed by tfry into branch 'master'.

Use a single means for handling sub-command loops, only

M  +19   -29   rkward/rbackend/rkrbackend.cpp
M  +1    -5    rkward/rbackend/rkrbackend.h
M  +3    -4    rkward/rbackend/rkrbackendprotocol_shared.h
M  +70   -74   rkward/rbackend/rkrinterface.cpp
M  +1    -1    rkward/rbackend/rktransmitter.cpp

https://invent.kde.org/education/rkward/-/commit/6b3494a7dd2e54163fe2f4d908d0a9f670efd748

diff --git a/rkward/rbackend/rkrbackend.cpp b/rkward/rbackend/rkrbackend.cpp
index 282abacbd..6577dca98 100644
--- a/rkward/rbackend/rkrbackend.cpp
+++ b/rkward/rbackend/rkrbackend.cpp
@@ -1427,30 +1427,31 @@ RCommandProxy *RKRBackend::commandFinished(FetchCommandMode fetch_next, ObjectUp
 		current_command = current_command->outer_command;
 	}
 
-	return RKRBackend::this_pointer->handleRequest(&req, false);
+	return RKRBackend::this_pointer->handleRequest(&req);
 }
 
-RCommandProxy *RKRBackend::handleRequest(RBackendRequest *request, bool mayHandleSubstack) {
+RCommandProxy *RKRBackend::handleRequest(RBackendRequest *request) {
 	RK_TRACE(RBACKEND);
 	RK_ASSERT(request);
 
-	// See docs for RBackendRequest for hints to make sense of this mess (and eventually to fix it)
-
-	RKRBackendProtocolBackend::instance()->sendRequest(request);
-	if (request->subcommandrequest) {
-		handleRequest2(request->subcommandrequest, true);
-	}
-	return handleRequest2(request, mayHandleSubstack);
-}
-
-RCommandProxy *RKRBackend::handleRequest2(RBackendRequest *request, bool mayHandleSubstack) {
-	RK_TRACE(RBACKEND);
-
+	// the logic is still a bit convoluted, here, for historical reasons: Subcommand requests are already sent as part of their parent request
+	if (request->type != RBackendRequest::SubcommandRequest) RKRBackendProtocolBackend::instance()->sendRequest(request);
 	if ((!request->synchronous) && (!isKilled())) {
 		RK_ASSERT(!request->subcommandrequest);
 		return nullptr;
 	}
 
+	// If the request allows subcommands doRCallRequest(..., SynchronousWithSubcommands),
+	// recurse into those, first
+	if (request->subcommandrequest) {
+		auto command = handleRequest(request->subcommandrequest);
+		while (command) {
+			runCommand(command);
+			command = commandFinished(FetchNextCommand, NoCheckObjectUpdatesNeeded);
+		}
+		handleDeferredInterrupts();
+	}
+
 	int i = 0;
 	while (!request->done) {
 		if (killed) return nullptr;
@@ -1485,16 +1486,7 @@ RCommandProxy *RKRBackend::handleRequest2(RBackendRequest *request, bool mayHand
 		}
 	}
 
-	if (!mayHandleSubstack) return command;
-
-	while (command) {
-		runCommand(command);
-		command = commandFinished(FetchNextCommand, NoCheckObjectUpdatesNeeded);
-	};
-
-	handleDeferredInterrupts();
-
-	return nullptr;
+	return command;
 }
 
 GenericRRequestResult RKRBackend::doRCallRequest(const QString &call, const QVariant &params, RequestFlags flags) {
@@ -1506,7 +1498,7 @@ GenericRRequestResult RKRBackend::doRCallRequest(const QString &call, const QVar
 	if (!params.isNull()) request.params[QStringLiteral("args")] = params;
 	if (flags == SynchronousWithSubcommands) {
 		request.params[QStringLiteral("cid")] = current_command->id;
-		request.subcommandrequest = new RBackendRequest(true, RBackendRequest::OtherRequest);
+		request.subcommandrequest = new RBackendRequest(true, RBackendRequest::SubcommandRequest);
 	}
 	handleRequest(&request);
 	delete request.subcommandrequest;
@@ -1520,7 +1512,7 @@ void RKRBackend::initialize(const QString &locale_dir, bool setup) {
 	repl_status.eval_depth++;
 	{
 		RBackendRequest req(true, RBackendRequest::CommandOut); // fetch the first command (a dummy)
-		handleRequest(&req, false);
+		handleRequest(&req);
 	}
 	RK_ASSERT(current_command);
 
@@ -1563,11 +1555,9 @@ void RKRBackend::initialize(const QString &locale_dir, bool setup) {
 		<p><b>You should quit RKWard, now, and fix your installation</b>. For help with that, see <a href=\"https://rkward.kde.org/compiling\">https://rkward.kde.org/compiling</a>.</p></p>\n"));
 	}
 
-	RBackendRequest req(true, RBackendRequest::Started);
-	req.params[QStringLiteral("message")] = QVariant(error_messages);
+	doRCallRequest(QStringLiteral("rstarted"), QVariant(error_messages), SynchronousWithSubcommands);
 	// blocks until RKWard is set to go (esp, it has displayed startup error messages, etc.)
 	// in fact, a number of sub-commands are run while handling this request!
-	handleRequest(&req);
 
 	RK_ASSERT(current_command);
 	commandFinished(FetchNextCommand, CheckObjectUpdatesNeeded); // the fake startup command
diff --git a/rkward/rbackend/rkrbackend.h b/rkward/rbackend/rkrbackend.h
index 7f101ffb9..97d9b88b3 100644
--- a/rkward/rbackend/rkrbackend.h
+++ b/rkward/rbackend/rkrbackend.h
@@ -86,7 +86,7 @@ class RKRBackend : public RKROutputBuffer {
 	@returns a pointer to the RCommandProxy-instance that was created and used, internally. You can query this pointer for status and data. Be sure to delete it, when done. */
 	RCommandProxy *runDirectCommand(const QString &command, RCommand::CommandTypes datatype);
 
-	void handleRequest(RBackendRequest *request) { handleRequest(request, true); };
+	RCommandProxy *handleRequest(RBackendRequest *request);
 
 	enum RequestFlags {
 		Asynchronous,
@@ -185,10 +185,6 @@ class RKRBackend : public RKROutputBuffer {
 
 	bool graphicsEngineMismatchMessage(int compiled_version, int runtime_version);
 
-  protected:
-	RCommandProxy *handleRequest(RBackendRequest *request, bool mayHandleSubstack);
-	RCommandProxy *handleRequest2(RBackendRequest *request, bool mayHandleSubstack);
-
   private:
 	int stdout_stderr_fd;
 	/** set up R standard callbacks */
diff --git a/rkward/rbackend/rkrbackendprotocol_shared.h b/rkward/rbackend/rkrbackendprotocol_shared.h
index 0336a251b..4495bdf8e 100644
--- a/rkward/rbackend/rkrbackendprotocol_shared.h
+++ b/rkward/rbackend/rkrbackendprotocol_shared.h
@@ -35,15 +35,14 @@ class RBackendRequest {
 		EditFiles,
 		ReadLine,   // 4
 		CommandOut, /**< Request the next command, and notify about the result of the previus. TODO split. */
-		Started,
-		RCallRequest, // 7
+		RCallRequest,
 		SetParamsFromBackend,
-		Debugger,
+		Debugger,                  // 8
 		Output,                    /**< A piece of output. Note: If the backend runs in a single process, output is handled in a pull fashion, instead of using requests. */
 		Interrupt,                 /**< Interrupt evaluation. This request type originates in the frontend, not the backend. */
 		PriorityCommand,           /**< Send a command to be run during R's event processing. This request type originates in the frontend, not the backend. */
 		OutputStartedNotification, /**< Only used in the frontend: Notification that a new bit of output has arrived. Used to trigger flushing after a timeout. */
-		OtherRequest               /**< Any other type of request. Note: which requests are in the enum, and which are not has mostly historical reasons. @see params */
+		SubcommandRequest          /**< For doRCall(..., SynchronousWithSubcommands) */
 	};
 
 	RBackendRequest(bool synchronous, RCallbackType type);
diff --git a/rkward/rbackend/rkrinterface.cpp b/rkward/rbackend/rkrinterface.cpp
index ec2241e98..b41cf16e9 100644
--- a/rkward/rbackend/rkrinterface.cpp
+++ b/rkward/rbackend/rkrinterface.cpp
@@ -387,79 +387,6 @@ void RInterface::handleRequest(RBackendRequest *request) {
 		if (subcommandrequest) closeChain(in_chain);
 
 		RKRBackendProtocolFrontend::setRequestCompleted(request);
-	} else if (request->type == RBackendRequest::Started) {
-		// The backend thread has finished basic initialization, but we still have more to do...
-		backend_error.message.append(request->params[QStringLiteral("message")].toString());
-
-		command_requests.append(request);
-		RCommandChain *chain = openSubcommandChain(runningCommand());
-
-		runStartupCommand(new RCommand(QStringLiteral("paste (R.version[c (\"major\", \"minor\")], collapse=\".\")\n"), RCommand::GetStringVector | RCommand::App | RCommand::Sync), chain,
-		                  [](RCommand *command) {
-			                  RK_ASSERT(command->getDataType() == RData::StringVector);
-			                  RK_ASSERT(command->getDataLength() == 1);
-			                  RKSessionVars::setRVersion(command->stringVector().value(0));
-		                  });
-
-		if (qEnvironmentVariableIsSet("APPDIR")) {
-			// Running inside an AppImage. As soon as R has started, it should behave as if running in the main (system) environment (esp. when calling helper binaries such as wget or gcc).
-			// Unset any paths starting with APPDIR, _except_ those inside R_HOME.
-			runStartupCommand(new RCommand(QStringLiteral("local({\n"
-			                                              "	appdir <- Sys.getenv(\"APPDIR\")\n"
-			                                              "	fix <- function(key) {\n"
-			                                              "		paths <- strsplit(Sys.getenv(key), \":\", fixed=TRUE)[[1]]\n"
-			                                              "		paths <- paths[!(startsWith(paths, appdir) & !startsWith(paths, R.home()))]\n"
-			                                              "		patharg <- list(paste(paths, collapse=\":\"))\n"
-			                                              "		names(patharg) <- key\n"
-			                                              "		do.call(Sys.setenv, patharg)\n"
-			                                              "	}\n"
-			                                              "	fix(\"LD_LIBRARY_PATH\")\n"
-			                                              "	fix(\"PATH\")\n"
-			                                              "})\n"),
-			                               RCommand::App | RCommand::Sync),
-			                  chain, [](RCommand *) {});
-		}
-
-		// find out about standard library locations
-		runStartupCommand(new RCommand(QStringLiteral("c(path.expand(Sys.getenv(\"R_LIBS_USER\")), .libPaths())\n"), RCommand::GetStringVector | RCommand::App | RCommand::Sync), chain,
-		                  [this](RCommand *command) {
-			                  RK_ASSERT(command->getDataType() == RData::StringVector);
-			                  RKSettingsModuleRPackages::r_libs_user = command->stringVector().value(0);
-			                  RKSettingsModuleRPackages::defaultliblocs = command->stringVector().mid(1);
-
-			                  RCommandChain *chain = command->parent;
-			                  RK_ASSERT(chain);
-			                  RK_ASSERT(!chain->isClosed());
-
-			                  // apply user configurable run time options
-			                  auto runtimeopt_callback = [](RCommand *) {}; // No special handling. Any failure will be recorded with runStartupCommand().
-			                  QStringList commands = RKSettingsModuleR::makeRRunTimeOptionCommands() + RKSettingsModuleRPackages::makeRRunTimeOptionCommands() + RKSettingsModuleOutput::makeRRunTimeOptionCommands() + RKSettingsModuleGraphics::makeRRunTimeOptionCommands();
-			                  for (QStringList::const_iterator it = commands.cbegin(); it != commands.cend(); ++it) {
-				                  runStartupCommand(new RCommand(*it, RCommand::App | RCommand::Sync), chain, runtimeopt_callback);
-			                  }
-			                  // initialize output file
-			                  RKOutputDirectory::getCurrentOutput(chain);
-
-			                  // Workaround for https://bugs.kde.org/show_bug.cgi?id=421958
-			                  if (RKSessionVars::compareRVersion(QStringLiteral("4.0.0")) < 1 && RKSessionVars::compareRVersion(QStringLiteral("4.0.1")) > 0) {
-				                  runStartupCommand(new RCommand(QStringLiteral("if(compiler::enableJIT(-1) > 2) compiler::enableJIT(2)\n"), RCommand::App | RCommand::Sync), chain, runtimeopt_callback);
-			                  }
-
-			                  closeChain(chain);
-		                  });
-
-		// start help server / determined help base url
-		runStartupCommand(new RCommand(QStringLiteral(".rk.getHelpBaseUrl ()\n"), RCommand::GetStringVector | RCommand::App | RCommand::Sync), chain,
-		                  [](RCommand *command) {
-			                  RK_ASSERT(command->getDataType() == RData::StringVector);
-			                  RK_ASSERT(command->getDataLength() == 1);
-			                  RKSettingsModuleR::help_base_url = command->stringVector().value(0);
-		                  });
-
-		QString cd_to = RKSettingsModuleGeneral::initialWorkingDirectory();
-		if (cd_to.isEmpty()) cd_to = QDir::currentPath();
-		if (cd_to.isEmpty()) cd_to = QStringLiteral("."); // we must be in a non-existent dir. cd'ing to "." will cause us to sync with the backend
-		RInterface::issueCommand(new RCommand(u"setwd("_s + RObject::rQuote(cd_to) + u")\n"_s, RCommand::App | RCommand::Sync), chain);
 	} else {
 		processRBackendRequest(request);
 	}
@@ -816,6 +743,75 @@ GenericRRequestResult RInterface::processRCallRequest(const QString &call, const
 		dialog->addRCommand(command, true);
 		issueCommand(command, in_chain);
 		dialog->doNonModal(true);
+	} else if (call == QLatin1String("rstarted")) {
+		// The backend thread has finished basic initialization, but we still have more to do...
+		backend_error.message.append(args.toString());
+
+		runStartupCommand(new RCommand(QStringLiteral("paste (R.version[c (\"major\", \"minor\")], collapse=\".\")\n"), RCommand::GetStringVector | RCommand::App | RCommand::Sync), in_chain,
+		                  [](RCommand *command) {
+			                  RK_ASSERT(command->getDataType() == RData::StringVector);
+			                  RK_ASSERT(command->getDataLength() == 1);
+			                  RKSessionVars::setRVersion(command->stringVector().value(0));
+		                  });
+
+		if (qEnvironmentVariableIsSet("APPDIR")) {
+			// Running inside an AppImage. As soon as R has started, it should behave as if running in the main (system) environment (esp. when calling helper binaries such as wget or gcc).
+			// Unset any paths starting with APPDIR, _except_ those inside R_HOME.
+			runStartupCommand(new RCommand(QStringLiteral("local({\n"
+			                                              "	appdir <- Sys.getenv(\"APPDIR\")\n"
+			                                              "	fix <- function(key) {\n"
+			                                              "		paths <- strsplit(Sys.getenv(key), \":\", fixed=TRUE)[[1]]\n"
+			                                              "		paths <- paths[!(startsWith(paths, appdir) & !startsWith(paths, R.home()))]\n"
+			                                              "		patharg <- list(paste(paths, collapse=\":\"))\n"
+			                                              "		names(patharg) <- key\n"
+			                                              "		do.call(Sys.setenv, patharg)\n"
+			                                              "	}\n"
+			                                              "	fix(\"LD_LIBRARY_PATH\")\n"
+			                                              "	fix(\"PATH\")\n"
+			                                              "})\n"),
+			                               RCommand::App | RCommand::Sync),
+			                  in_chain, [](RCommand *) {});
+		}
+
+		// find out about standard library locations
+		runStartupCommand(new RCommand(QStringLiteral("c(path.expand(Sys.getenv(\"R_LIBS_USER\")), .libPaths())\n"), RCommand::GetStringVector | RCommand::App | RCommand::Sync), in_chain,
+		                  [this](RCommand *command) {
+			                  RK_ASSERT(command->getDataType() == RData::StringVector);
+			                  RKSettingsModuleRPackages::r_libs_user = command->stringVector().value(0);
+			                  RKSettingsModuleRPackages::defaultliblocs = command->stringVector().mid(1);
+
+			                  RCommandChain *chain = command->parent;
+			                  RK_ASSERT(chain);
+
+			                  // apply user configurable run time options
+			                  auto runtimeopt_callback = [](RCommand *) {}; // No special handling. Any failure will be recorded with runStartupCommand().
+			                  QStringList commands = RKSettingsModuleR::makeRRunTimeOptionCommands() + RKSettingsModuleRPackages::makeRRunTimeOptionCommands() + RKSettingsModuleOutput::makeRRunTimeOptionCommands() + RKSettingsModuleGraphics::makeRRunTimeOptionCommands();
+			                  for (QStringList::const_iterator it = commands.cbegin(); it != commands.cend(); ++it) {
+				                  runStartupCommand(new RCommand(*it, RCommand::App | RCommand::Sync), chain, runtimeopt_callback);
+			                  }
+			                  // initialize output file
+			                  RKOutputDirectory::getCurrentOutput(chain);
+
+			                  // Workaround for https://bugs.kde.org/show_bug.cgi?id=421958
+			                  if (RKSessionVars::compareRVersion(QStringLiteral("4.0.0")) < 1 && RKSessionVars::compareRVersion(QStringLiteral("4.0.1")) > 0) {
+				                  runStartupCommand(new RCommand(QStringLiteral("if(compiler::enableJIT(-1) > 2) compiler::enableJIT(2)\n"), RCommand::App | RCommand::Sync), chain, runtimeopt_callback);
+			                  }
+
+			                  closeChain(chain);
+		                  });
+
+		// start help server / determined help base url
+		runStartupCommand(new RCommand(QStringLiteral(".rk.getHelpBaseUrl ()\n"), RCommand::GetStringVector | RCommand::App | RCommand::Sync), in_chain,
+		                  [](RCommand *command) {
+			                  RK_ASSERT(command->getDataType() == RData::StringVector);
+			                  RK_ASSERT(command->getDataLength() == 1);
+			                  RKSettingsModuleR::help_base_url = command->stringVector().value(0);
+		                  });
+
+		QString cd_to = RKSettingsModuleGeneral::initialWorkingDirectory();
+		if (cd_to.isEmpty()) cd_to = QDir::currentPath();
+		if (cd_to.isEmpty()) cd_to = QStringLiteral("."); // we must be in a non-existent dir. cd'ing to "." will cause us to sync with the backend
+		issueCommand(new RCommand(u"setwd("_s + RObject::rQuote(cd_to) + u")\n"_s, RCommand::App | RCommand::Sync), in_chain);
 	} else {
 		return GenericRRequestResult::makeError(i18n("Error: unrecognized request '%1'", call));
 	}
@@ -931,7 +927,7 @@ void RInterface::processRBackendRequest(RBackendRequest *request) {
 			if (report) reportFatalError(); // TODO: Reporting should probably be moved to RKWardMinaWindow, entirely
 		}
 	} else {
-		RK_ASSERT(false);
+		RK_DEBUG(RBACKEND, DL_ERROR, "Bad request type %d", type);
 	}
 
 	RKRBackendProtocolFrontend::setRequestCompleted(request);
diff --git a/rkward/rbackend/rktransmitter.cpp b/rkward/rbackend/rktransmitter.cpp
index 0fc65a34e..9a52e16b0 100644
--- a/rkward/rbackend/rktransmitter.cpp
+++ b/rkward/rbackend/rktransmitter.cpp
@@ -48,7 +48,7 @@ static T readS(QDataStream &stream) {
 RBackendRequest *RKRBackendSerializer::unserialize(QDataStream &stream) {
 	RK_TRACE(RBACKEND);
 
-	RBackendRequest *request = new RBackendRequest(false, RBackendRequest::OtherRequest); // will be overwritten
+	RBackendRequest *request = new RBackendRequest(false, RBackendRequest::SubcommandRequest); // will be overwritten
 	RBackendRequest::_id--;
 
 	request->id = readS<qint32>(stream);



More information about the rkward-tracker mailing list