[education/rkward] rkward: Backend select function mostly complete

Thomas Friedrichsmeier null at kde.org
Sat Jun 8 23:21:01 BST 2024


Git commit 1aa3c4b1f549577a831602bb3657ed7597e1c931 by Thomas Friedrichsmeier.
Committed on 08/06/2024 at 22:10.
Pushed by tfry into branch 'master'.

Backend select function mostly complete

(Selection still needs to be saved, and restored; selection mechanisms need to be reconciled)

M  +1    -3    rkward/autotests/core_test.cpp
M  +133  -63   rkward/dialogs/rksetupwizard.cpp
M  +1    -0    rkward/dialogs/rksetupwizard.h
M  +1    -0    rkward/rbackend/rksessionvars.h
M  +45   -35   rkward/rkward.cpp
M  +4    -0    rkward/rkward.h

https://invent.kde.org/education/rkward/-/commit/1aa3c4b1f549577a831602bb3657ed7597e1c931

diff --git a/rkward/autotests/core_test.cpp b/rkward/autotests/core_test.cpp
index 53aa3ebe1..34a6f5ced 100644
--- a/rkward/autotests/core_test.cpp
+++ b/rkward/autotests/core_test.cpp
@@ -413,14 +413,12 @@ private Q_SLOTS:
 	}
 
 	void restartRBackend() {
-		auto restart_action = RKWardMainWindow::getMain()->actionCollection()->action("restart_r");
-		QVERIFY(restart_action != nullptr);
 		RInterface::issueCommand(new RCommand("x <- 1", RCommand::User));
 		waitForAllFinished();
 		QVERIFY(RObjectList::getGlobalEnv()->findObject("x"));
 
 		QPointer<RInterface> oldiface = RInterface::instance();
-		restart_action->trigger();
+		RKWardMainWindow::getMain()->triggerBackendRestart(false);
 		QElapsedTimer t;
 		t.start();
 		while (oldiface) {  // action may be delayed until next event processing
diff --git a/rkward/dialogs/rksetupwizard.cpp b/rkward/dialogs/rksetupwizard.cpp
index 45e30000a..0183265a8 100644
--- a/rkward/dialogs/rksetupwizard.cpp
+++ b/rkward/dialogs/rksetupwizard.cpp
@@ -97,6 +97,110 @@ static auto iconForStatus(RKSetupWizardItem::Status status) {
 	return (QIcon::fromTheme(icon_id).pixmap(32, 32));  // TODO: Correct way to not hardcode size?
 }
 
+class RBackendStatusWidget : public QWidget {
+public:
+	RBackendStatusWidget(QWidget *parent) : QWidget(parent), anim(nullptr) {
+		auto l = new QVBoxLayout(this);
+		rinst_label = new QLabel();
+		l->addWidget(rinst_label);
+		auto h = new QHBoxLayout();
+		l->addLayout(h);
+		rstatus_label = RKCommonFunctions::wordWrappedLabel(QString());
+		rstatus_icon = new QLabel();
+		h->addWidget(rstatus_icon);
+		h->addWidget(rstatus_label);
+		h->setStretch(1, 2);
+		update();
+	}
+	void update() {
+		if (RInterface::instance()->backendIsDead()) {
+			rstatus_icon->setPixmap(iconForStatus(RKSetupWizardItem::Error));
+			rstatus_label->setText(i18n("<b>R backend has crashed.</b>"));
+			if (anim) {
+				delete anim;
+				anim = nullptr;
+			}
+		} else if (RInterface::instance()->backendIsIdle()) {
+			QString statustext = i18n("<b>R version %1 started, successfully.</b>", RKSessionVars::RVersion(false));
+			if (RKSessionVars::isPathInAppImage(RKSessionVars::RBinary())) {
+				rstatus_icon->setPixmap(iconForStatus(RKSetupWizardItem::Warning));
+				statustext.append(i18n("<p>You are using the R version bundled inside the RKWard AppImage.</p>"
+				                       "<p>This version comes with several technical limitations. Importantly, "
+				                       "yu will not be able to install most R addon packages. "
+				                       "In general, it is therefore recommended to select a system-installed "
+				                       "version of R, instead, below.</p>"));
+			} else {
+				rstatus_icon->setPixmap(iconForStatus(RKSetupWizardItem::Good));
+			}
+			rstatus_label->setText(statustext);
+			if (anim) {
+				delete anim;
+				anim = nullptr;
+			}
+		} else {
+			rinst_label->setText(i18n("RKWard is currently using the R installation at <nobr>%1</nobr>.", RKSessionVars::RBinary()));
+			rstatus_label->setText(i18n("<b>Waiting for R backend...</b>"));
+			if (!anim) anim = RKStandardIcons::busyAnimation(this, [this](const QIcon& icon) {
+				rstatus_icon->setPixmap(icon.pixmap(32, 32));
+			});
+		}
+	}
+private:
+	QLabel *rstatus_label, *rstatus_icon, *rinst_label;
+	QTimer *anim;
+};
+
+class RBackendSelectionWidget : public QGroupBox {
+public:
+	RBackendSelectionWidget(QWidget *parent) : QGroupBox(parent) {
+		group = new QButtonGroup(this);
+		auto l = new QVBoxLayout(this);
+		bl = new QVBoxLayout();
+		l->addLayout(bl);
+
+		auto button = new QRadioButton(i18n("Use R at:")); l->addWidget(button); group->addButton(button, -1);
+		req = new KUrlRequester(); l->addWidget(req);
+		req->setPlaceholderText(i18n("Select another R executable"));
+		req->setEnabled(false);
+		req->setWindowTitle(i18n("Select R executable"));
+		connect(button, &QAbstractButton::toggled, req, &QWidget::setEnabled);
+	}
+
+	void updateOptions() {
+		// clear previous buttons, if any
+		for (int i = 0; i < r_installations.size(); ++i) {
+			delete (group->button(i));
+		}
+
+		r_installations = RKSessionVars::findRInstallations();
+		r_installations.removeAll(RKSessionVars::RBinary());
+		r_installations.prepend(RKSessionVars::RBinary());
+		for(int i = 0; i < r_installations.size(); ++i) {
+			addButton(i18n("Use R at %1", r_installations[i]), i);
+		}
+		auto button = group->button(0);
+		if (RInterface::instance()->backendIsDead()) button->setText(i18n("Attempt to restart R at %1", RKSessionVars::RBinary()));
+		else button->setText(i18n("Keep current version"));
+		button->setChecked(true);
+	}
+	QRadioButton *addButton(const QString &text, int index) {
+		auto button = new QRadioButton(text);
+		bl->addWidget(button);
+		group->addButton(button, index);
+		return button;
+	}
+	QString selectedOpt() {
+		int index = group->checkedId();
+		if (index >= 0) return r_installations[index];
+		return req->text();
+	}
+	QButtonGroup *group;
+private:
+	QVBoxLayout *bl;
+	QStringList r_installations;
+	KUrlRequester *req;
+};
+
 void RKSetupWizardItem::createWidget(QGridLayout *layout, int row) {
 	auto label = new QLabel();
 	label->setPixmap(iconForStatus(status));
@@ -168,7 +272,7 @@ RKSetupWizardItem* makeSoftwareCheck(const QString &exename, const QString& expl
 	return ret;
 }
 
-RKSetupWizard::RKSetupWizard(QWidget* parent, InvokationReason reason, const QList<RKSetupWizardItem*> &settings_items) : KAssistantDialog(parent) {
+RKSetupWizard::RKSetupWizard(QWidget* parent, InvokationReason reason, const QList<RKSetupWizardItem*> &settings_items) : KAssistantDialog(parent), reason(reason) {
 	RK_TRACE (DIALOGS);
 
 	// Cover page
@@ -247,76 +351,42 @@ RKSetupWizard::RKSetupWizard(QWidget* parent, InvokationReason reason, const QLi
 	page = new RKSetupWizardPage(this, i18n("R Backend"));
 	page->lazyInitOnce([this](RKSetupWizardPage *p) {
 		auto l = new QVBoxLayout(p);
-		l->addWidget(new QLabel(i18n("RKWard is currently using the R installation at <nobr>%1</nobr>.", RKSessionVars::RBinary())));
-		auto h = new QHBoxLayout();
-		l->addLayout(h);
-		auto rstatus_label = RKCommonFunctions::wordWrappedLabel(i18n("<b>Waiting for R backend...</b>"));
-		auto rstatus_icon = new QLabel();
-		auto anim = RKStandardIcons::busyAnimation(rstatus_icon, [rstatus_icon](const QIcon& icon) {
-			rstatus_icon->setPixmap(icon.pixmap(32, 32));
-		});
-		h->addWidget(rstatus_icon);
-		h->addWidget(rstatus_label);
-		h->setStretch(1, 2);
+		auto status = new RBackendStatusWidget(p);
+		auto select = new RBackendSelectionWidget(p);
+		l->addWidget(status);
+		l->addWidget(select);
 		l->addStretch();
 		auto pageref = p->ref;
 		setValid(pageref, false);
-		auto statuslambda = [this, pageref, rstatus_label, rstatus_icon, anim, l]() {
-			if (!(RInterface::instance()->backendIsDead() || RInterface::instance()->backendIsIdle())) return;
-
-			anim->stop();
-			bool current_valid = true;
-			if (RInterface::instance()->backendIsDead()) {
-				rstatus_icon->setPixmap(iconForStatus(RKSetupWizardItem::Error));
-				rstatus_label->setText(i18n("<b>R backend has crashed.</b>"));
-				current_valid = false;
-			} else {
-				RK_ASSERT(RInterface::instance()->backendIsIdle());
-				QString statustext = i18n("<b>R version %1 started, successfully.</b>", RKSessionVars::RVersion(false));
-				if (RKSessionVars::isPathInAppImage(RKSessionVars::RBinary())) {
-					rstatus_icon->setPixmap(iconForStatus(RKSetupWizardItem::Warning));
-					statustext.append(i18n("<p>You are using the R version bundled inside the RKWard AppImage.</p>"
-					                       "<p>This version comes with several technical limitations. Importantly, "
-					                       "yu will not be able to install most R addon packages. "
-					                       "In general, it is therefore recommended to select a system-installed "
-					                       "version of R, instead, below.</p>"));
-				} else {
-					rstatus_icon->setPixmap(iconForStatus(RKSetupWizardItem::Good));
-				}
-				rstatus_label->setText(statustext);
-				setValid(pageref, true);
-			}
 
-			// Selector widget for R installation. Need to delete/recreate this on each page enter
-			delete (property("sel").value<QWidget*>());
-			auto box = new QGroupBox(i18n("R version to use"));
-			setProperty("sel", QVariant::fromValue(box));
-
-			auto group = new QButtonGroup(box);
-			l->addWidget(box);
-			auto bl = new QVBoxLayout(box);
-			QStringList r_installations = RKSessionVars::findRInstallations();
-			r_installations.removeAll(RKSessionVars::RBinary());
-			auto button = new QRadioButton(i18n("Keep current version"), box); bl->addWidget(button); group->addButton(button, 0);
-			if (!current_valid) button->setText(i18n("Attempt to restart R at %1", RKSessionVars::RBinary()));
-			button->setChecked(true);
-
-			for(const QString &inst : std::as_const(r_installations)) {
-				button = new QRadioButton(i18n("Use R at %1", inst), box); bl->addWidget(button); group->addButton(button);
+		auto statuslambda = [this, pageref, status, select]() {
+			status->update();
+			if (!(RInterface::instance()->backendIsDead() || RInterface::instance()->backendIsIdle())) {
+				select->hide();
+				return;
 			}
-			button = new QRadioButton(i18n("Use R at:"), box); bl->addWidget(button); group->addButton(button);
-			auto req = new KUrlRequester(); bl->addWidget(req);
-			req->setPlaceholderText(i18n("Select another R executable"));
-			req->setEnabled(false);
-			req->setWindowTitle(i18n("Select R executable"));
-			connect(button, &QAbstractButton::toggled, req, &QWidget::setEnabled);
-
-			next_callbacks.insert(pageref, [group]() -> bool {
-				bool proceed = (group->checkedId() == 0) && RInterface::instance()->backendIsIdle();
-				return proceed;
+			select->updateOptions();
+			select->show();
+			setValid(pageref, true);
+
+			next_callbacks.insert(pageref, [select, pageref, this]() -> bool {
+				bool restart_needed = (select->group->checkedId() != 0) || RInterface::instance()->backendIsDead();
+				if (restart_needed) {
+					RKSessionVars::r_binary = select->selectedOpt();
+					if (RKWardMainWindow::getMain()->triggerBackendRestart(this->reason == ManualCheck)) {
+						select->hide();
+						setValid(pageref, false);
+					}
+				}
+				return !restart_needed;
 			});
 		};
 		connect(RInterface::instance(), &RInterface::backendStatusChanged, this, statuslambda);
+		// when restarting the backend, the RInterface::instance() itself is exchanged
+		connect(RKWardMainWindow::getMain(), &RKWardMainWindow::backendCreated, this, [this, statuslambda]() {
+			connect(RInterface::instance(), &RInterface::backendStatusChanged, this, statuslambda);
+			statuslambda();
+		});
 		statuslambda();
 	});
 
diff --git a/rkward/dialogs/rksetupwizard.h b/rkward/dialogs/rksetupwizard.h
index 1ad537134..a141e85a5 100644
--- a/rkward/dialogs/rksetupwizard.h
+++ b/rkward/dialogs/rksetupwizard.h
@@ -46,6 +46,7 @@ friend class RKSetupWizardPage;
 	QMap<KPageWidgetItem*, std::function<bool()>> next_callbacks;
 	QList<RKSetupWizardItem*> items;
 	bool reinstallation_required;
+	InvokationReason reason;
 };
 
 class QComboBox;
diff --git a/rkward/rbackend/rksessionvars.h b/rkward/rbackend/rksessionvars.h
index 79a2d64f6..332fc0e63 100644
--- a/rkward/rbackend/rksessionvars.h
+++ b/rkward/rbackend/rksessionvars.h
@@ -54,6 +54,7 @@ private:
 	static QString appimagedir;
 friend int main(int, char**);
 friend class RKWardCoreTest;
+friend class RKSetupWizard;
 	static QString r_binary;
 };
 
diff --git a/rkward/rkward.cpp b/rkward/rkward.cpp
index 73fa7329a..a3b2a9c2a 100644
--- a/rkward/rkward.cpp
+++ b/rkward/rkward.cpp
@@ -388,6 +388,7 @@ void RKWardMainWindow::startR () {
 	}
 
 	RInterface::create();
+	Q_EMIT(backendCreated());
 	connect(RInterface::instance(), &RInterface::backendStatusChanged, this, &RKWardMainWindow::setRStatus);
 	RObjectList::init();
 
@@ -619,43 +620,52 @@ void RKWardMainWindow::initActions() {
 	restart_r = actionCollection()->addAction("restart_r");
 	restart_r->setIcon(QIcon::fromTheme("view-refresh"));
 	restart_r->setText(i18n("Restart R Backend"));
-	connect(restart_r, &QAction::triggered, this, [this]() {
-		QString message = i18n("<p>This feature is primarily targetted at advanced users working on scripts or packages. Please proceed with caution.</p><p><b>All unsaved data in this workspace will be lost!</b> All data editors, and graphics windows will be closed.</p><p>Are you sure you want to proceed?</p>");
-		if (suppressModalDialogsForTesting() || (KMessageBox::warningContinueCancel(this, message, i18n("Restart R backend"), KGuiItem(i18n("Restart R backend"))) == KMessageBox::Continue)) {
-			bool forced = RInterface::instance()->backendIsDead();
-			while (!RInterface::instance()->backendIsDead() && !RInterface::instance()->backendIsIdle()) {
-				RK_DEBUG(APP, DL_DEBUG, "Backend not idle while restart requested.");
-				message = i18n("<p>One or more operations are pending.</p><p>If you have recently chosen to save your workspace, and you see this message, <b>your data may not be saved, yet!</b></p><p>How do you want to proceed?</p>");
-				auto res = KMessageBox::warningTwoActionsCancel(this, message, i18n("R commands still pending"), KGuiItem(i18n("Force restart now")), KGuiItem(i18n("Check again")), KGuiItem(i18n("Cancel restarting")));
-				if (res == KMessageBox::PrimaryAction) {
-					forced = true;
-					break;
-				} else if (res == KMessageBox::Cancel) {
-					return;
-				}
-				// KMessageBox::No means to loop: Commands may have finished, meanwhile. This is not really pretty, a proper progress control would be nicer,
-				// but then, this is a corner case to a feature that is not really targetted at a mainstream audience, anyway.
-			}
+	connect(restart_r, &QAction::triggered, this, &RKWardMainWindow::triggerBackendRestart);
+}
 
-			RKWorkplace::mainWorkplace()->closeAll(RKMDIWindow::X11Window);
-			slotCloseAllEditors();
-			auto restart_now = [this]() {
-				RK_DEBUG(APP, DL_DEBUG, "Backend restart now");
-				delete RInterface::instance();  // NOTE: Do not use deleteLater(), here. It is important to fully tear down the old backend, before creating the new one,
-				                                //       as code is written around the assumption that RInterface and friends are singletons. (RInterface::instance(), etc.)
-				RKWorkplace::mainWorkplace()->setWorkspaceURL(QUrl());
-				startR();
-			};
-			if (forced) {
-				RKConsole::mainConsole()->resetConsole();
-				restart_now();
-			} else {
-				RCommand *c = new RCommand(QString("# Quit (restarting)"), RCommand::App | RCommand::EmptyCommand | RCommand::QuitCommand);
-				c->whenFinished(this, [this, restart_now]() { QTimer::singleShot(0, this, restart_now); });
-				RInterface::issueCommand(c);
-			}
+bool RKWardMainWindow::triggerBackendRestart(bool promptsave) {
+	RK_TRACE (APP);
+
+	promptsave = promptsave && !suppressModalDialogsForTesting();
+	QString message = i18n("<p><b>Restarting the backend will discard all unsaved data in this workspace!</b> All data editors, and graphics windows will be closed.</p><p>Are you sure you want to proceed?</p>");
+	if (promptsave && (KMessageBox::warningContinueCancel(this, message, i18n("Restart R backend"), KGuiItem(i18n("Restart R backend"))) != KMessageBox::Continue)) {
+		return false;
+	}
+
+	bool forced = RInterface::instance()->backendIsDead();
+	while (!RInterface::instance()->backendIsDead() && !RInterface::instance()->backendIsIdle()) {
+		RK_DEBUG(APP, DL_DEBUG, "Backend not idle while restart requested.");
+		message = i18n("<p>One or more operations are pending.</p><p>If you have recently chosen to save your workspace, and you see this message, <b>your data may not be saved, yet!</b></p><p>How do you want to proceed?</p>");
+		auto res = KMessageBox::warningTwoActionsCancel(this, message, i18n("R commands still pending"), KGuiItem(i18n("Force restart now")), KGuiItem(i18n("Check again")), KGuiItem(i18n("Cancel restarting")));
+		if (res == KMessageBox::PrimaryAction) {
+			forced = true;
+			break;
+		} else if (res == KMessageBox::Cancel) {
+			return false;
 		}
-	});
+		// KMessageBox::No means to loop: Commands may have finished, meanwhile. This is not really pretty, a proper progress control would be nicer,
+		// but then, this is a corner case to a feature that is not really targetted at a mainstream audience, anyway.
+	}
+
+	RKWorkplace::mainWorkplace()->closeAll(RKMDIWindow::X11Window);
+	slotCloseAllEditors();
+	auto restart_now = [this]() {
+		RK_DEBUG(APP, DL_DEBUG, "Backend restart now");
+		delete RInterface::instance();  // NOTE: Do not use deleteLater(), here. It is important to fully tear down the old backend, before creating the new one,
+						//       as code is written around the assumption that RInterface and friends are singletons. (RInterface::instance(), etc.)
+		RKWorkplace::mainWorkplace()->setWorkspaceURL(QUrl());
+		startR();
+	};
+	if (forced) {
+		RKConsole::mainConsole()->resetConsole();
+		restart_now();
+	} else {
+		RCommand *c = new RCommand(QString("# Quit (restarting)"), RCommand::App | RCommand::EmptyCommand | RCommand::QuitCommand);
+		c->whenFinished(this, [this, restart_now]() { QTimer::singleShot(0, this, restart_now); });
+		RInterface::issueCommand(c);
+	}
+
+	return true;
 }
 
 /*
diff --git a/rkward/rkward.h b/rkward/rkward.h
index 4636da6b8..3bb2d137c 100644
--- a/rkward/rkward.h
+++ b/rkward/rkward.h
@@ -68,6 +68,7 @@ protected:
 Q_SIGNALS:
 	void aboutToQuitRKWard ();
 	void tabForToolViewAdded(QWidget*, QWidget*);  // Needed from katepluginintegration
+	void backendCreated();
 public Q_SLOTS:
 	void setWorkspaceUnmodified () { setWorkspaceMightBeModified (false); };
 	/** open a workspace. If the current workspace is not empty, ask whether to save first.
@@ -119,6 +120,9 @@ public Q_SLOTS:
 /** HACK this is only to make the compiler happy with -Woverloaded-virtual */
 	void setCaption (const QString &dummy, bool) override { setCaption (dummy); };
 	void openUrlsFromCommandLineOrExternal(bool no_warn_external, QStringList urls);
+/** Trigger restart of backend. Does not wait for restart to actually complete.
+ *  Return true, if restart was triggered, false if restart has been cancelled */
+	bool triggerBackendRestart(bool promptsave=true);
 private Q_SLOTS:
 	void partChanged (KParts::Part *new_part);
 private:



More information about the rkward-tracker mailing list