[education/rkward] rkward/core: Port RObject away from RCommandReceiver

Thomas Friedrichsmeier null at kde.org
Thu Jun 9 15:30:34 BST 2022


Git commit 052d9b0ed3234ac3c88d2ac3d8970a8defe9cd73 by Thomas Friedrichsmeier.
Committed on 09/06/2022 at 14:29.
Pushed by tfry into branch 'master'.

Port RObject away from RCommandReceiver

M  +0    -21   rkward/core/renvironmentobject.cpp
M  +1    -1    rkward/core/renvironmentobject.h
M  +78   -89   rkward/core/rkvariable.cpp
M  +0    -2    rkward/core/rkvariable.h
M  +70   -31   rkward/core/robject.cpp
M  +8    -6    rkward/core/robject.h
M  +22   -25   rkward/core/robjectlist.cpp
M  +1    -1    rkward/core/robjectlist.h

https://invent.kde.org/education/rkward/commit/052d9b0ed3234ac3c88d2ac3d8970a8defe9cd73

diff --git a/rkward/core/renvironmentobject.cpp b/rkward/core/renvironmentobject.cpp
index 62ca2ae6..d4078320 100644
--- a/rkward/core/renvironmentobject.cpp
+++ b/rkward/core/renvironmentobject.cpp
@@ -7,13 +7,11 @@ SPDX-License-Identifier: GPL-2.0-or-later
 
 #include "renvironmentobject.h"
 
-#include <kmessagebox.h>
 #include <KLocalizedString>
 
 #include "robjectlist.h"
 #include "rkpseudoobjects.h"
 #include "../rbackend/rkrinterface.h"
-#include "../settings/rksettingsmoduleobjectbrowser.h"
 #include "rkmodificationtracker.h"
 
 #include "../debug.h"
@@ -90,25 +88,6 @@ void REnvironmentObject::writeMetaData (RCommandChain *chain) {
 	RContainerObject::writeMetaData (chain);
 }
 
-void REnvironmentObject::updateFromR (RCommandChain *chain) {
-	RK_TRACE (OBJECTS);
-	if (type & PackageEnv) {
-		if (RKSettingsModuleObjectBrowser::isPackageBlacklisted (packageName ())) {
-			KMessageBox::information (0, i18n ("The package '%1' (probably you just loaded it) is currently blacklisted for retrieving structure information. Practically this means, the objects in this package will not appear in the object browser, and there will be no object name completion or function argument hinting for objects in this package.\nPackages will typically be blacklisted, if they contain huge amount of data, that would take too long to load. To unlist the package, visit Settings->Configure RKWard->Workspace.", packageName ()), i18n("Package blacklisted"), "packageblacklist" + packageName ());
-			return;
-		}
-	}
-
-	QString options;
-	if (type & GlobalEnv) options = ", envlevel=-1";	// in the .GlobalEnv recurse one more level
-	if (type & PackageEnv) options.append (", namespacename=" + rQuote (packageName ()));
-
-	RCommand *command = new RCommand (".rk.get.structure (" + getFullName (DefaultObjectNameOptions) + ", " + rQuote (getShortName ()) + options + ')', RCommand::App | RCommand::Sync | RCommand::GetStructuredData, QString (), this, ROBJECT_UDPATE_STRUCTURE_COMMAND);
-	RInterface::issueCommand (command, chain);
-
-	type |= Updating;
-}
-
 void REnvironmentObject::updateFromR(RCommandChain *chain, const QStringList &added_symbols, const QStringList &removed_symbols) {
 	RK_TRACE (OBJECTS);
 
diff --git a/rkward/core/renvironmentobject.h b/rkward/core/renvironmentobject.h
index ab9506dc..decb9d81 100644
--- a/rkward/core/renvironmentobject.h
+++ b/rkward/core/renvironmentobject.h
@@ -22,7 +22,7 @@ public:
 	REnvironmentObject (RContainerObject *parent, const QString &name);
 	~REnvironmentObject ();
 
-	void updateFromR (RCommandChain *chain) override;
+	using RObject::updateFromR;
 /** like updateFromR, but only update new / removed symbols from R. Theoretically this could be defined in RContainerObject, but the only use case is for environments. */
 	virtual void updateFromR (RCommandChain *chain, const QStringList &added_symbols, const QStringList &removed_symbols);
 
diff --git a/rkward/core/rkvariable.cpp b/rkward/core/rkvariable.cpp
index 1b85e8ee..02515894 100644
--- a/rkward/core/rkvariable.cpp
+++ b/rkward/core/rkvariable.cpp
@@ -17,8 +17,6 @@ SPDX-License-Identifier: GPL-2.0-or-later
 
 #include "rkmodificationtracker.h"
 
-#define GET_DATA_COMMAND 11
-
 #define MAX_PRECISION DBL_DIG
 
 #include "../debug.h"
@@ -105,92 +103,6 @@ void RKVariable::writeMetaData (RCommandChain *chain) {
 	RObject::writeMetaData (chain);
 }
 
-void RKVariable::rCommandDone (RCommand *command) {
-	RK_TRACE (OBJECTS);
-	
-	if (command->getFlags () == ROBJECT_UDPATE_STRUCTURE_COMMAND) {
-		RObject::rCommandDone (command);
-	} else if (command->getFlags () == GET_DATA_COMMAND) {
-		if (!data) return;	// this can happen, if the editor is closed while a data update is still queued.
-
-		// prevent resyncing of data
-		lockSyncing (true);
-
-		RK_ASSERT (command->getDataType () == RData::StructureVector);
-		RK_ASSERT (command->getDataLength () == 3);
-
-		RData::RDataStorage top = command->structureVector ();
-		RData *cdata = top.at (0);
-		RData *levels = top.at (1);
-		RData *invalids = top.at (2);
-
-		// set factor levels first
-		RK_ASSERT (levels->getDataType () == RData::StringVector);
-		QStringList new_levels = levels->stringVector ();
-		int levels_len = new_levels.size ();
-		RK_ASSERT (levels_len >= 1);
-		delete data->value_labels;
-		data->value_labels = new RObject::ValueLabels;
-		if ((levels_len == 1) && new_levels.at (0).isEmpty ()) {
-			// no levels
-		} else {
-			for (int i=0; i < levels_len; ++i) {
-				data->value_labels->insert (QString::number (i+1), new_levels.at (i));
-			}
-		}
-
-		// now set the data
-		RK_ASSERT (cdata->getDataLength () == (unsigned int) getLength ()); // not really a problem due to the line below, I'd still like to know if / when this happens.
-		extendToLength (cdata->getDataLength ());
-		if (cdata->getDataType () == RData::StringVector) {
-			setCharacterFromR (0, getLength () - 1, cdata->stringVector ());
-		} else if (cdata->getDataType () == RData::RealVector) {
-			setNumericFromR (0, getLength () - 1, cdata->realVector ());
-		} else if (cdata->getDataType () == RData::IntVector) {
-			RData::IntStorage int_data = cdata->intVector ();
-			unsigned int len = getLength ();
-			QVector<double> dd;
-			dd.reserve (len);
-			for (unsigned int i = 0; i < len; ++i) {
-				if (RInterface::isNaInt (int_data.at (i))) dd.append (NAN);
-				else dd.append ((double) int_data.at (i));
-			}
-			setNumericFromR (0, getLength () - 1, dd);
-		}
-
-		// now set the invalid fields (only if they are still NAs in the R data)
-		data->invalid_fields.clear ();
-		if (invalids->getDataLength () <= 1) {
-			// no invalids
-		} else {
-			RK_ASSERT (invalids->getDataType () == RData::StringVector);
-			QStringList invalids_list = invalids->stringVector ();
-			int invalids_length = invalids_list.size ();
-			RK_ASSERT ((invalids_length % 2) == 0);
-			int invalids_count = invalids_length / 2;
-			for (int i=0; i < invalids_count; ++i) {
-				int row = invalids_list.at (i).toInt () - 1;
-				if (data->cell_states[row] & RKVarEditData::NA) {	// NOTE: Do *not* use setText(), here. It tries too hard to set a valid value.
-					data->invalid_fields.insert (row, invalids_list.at (invalids_count + i));
-					data->cell_states[row] = RKVarEditData::Invalid;
-				}
-			}
-		}
-		data->previously_valid = data->invalid_fields.isEmpty ();
-		data->formatting_options = parseFormattingOptionsString (getMetaProperty ("format"));
-
-		ChangeSet *set = new ChangeSet (0, getLength (), true);
-		RKModificationTracker::instance()->objectDataChanged (this, set);
-		RKModificationTracker::instance()->objectMetaChanged (this);
-		type -= (type & NeedDataUpdate);
-		discardUnsyncedChanges ();
-		lockSyncing (false);
-	} else {
-		RK_ASSERT (false);
-	}
-}
-
-
 ////////////////////// BEGIN: data-handling //////////////////////////////
 #define ALLOC_STEP 2
 #define INITIAL_ALLOC 100
@@ -288,7 +200,84 @@ void RKVariable::updateDataFromR (RCommandChain *chain) {
 	RK_TRACE (OBJECTS);
 	if (!data) return;
 
-	RInterface::issueCommand (".rk.get.vector.data (" + getFullName () + ')', RCommand::App | RCommand::Sync | RCommand::GetStructuredData, QString (), this, GET_DATA_COMMAND, chain);
+	RCommand *c = new RCommand(".rk.get.vector.data (" + getFullName () + ')', RCommand::App | RCommand::Sync | RCommand::GetStructuredData);
+	whenCommandFinished(c, [this](RCommand* command) {
+		if (!data) return;	// this can happen, if the editor is closed while a data update is still queued.
+
+		// prevent resyncing of data
+		lockSyncing (true);
+
+		RK_ASSERT (command->getDataType () == RData::StructureVector);
+		RK_ASSERT (command->getDataLength () == 3);
+
+		RData::RDataStorage top = command->structureVector ();
+		RData *cdata = top.at (0);
+		RData *levels = top.at (1);
+		RData *invalids = top.at (2);
+
+		// set factor levels first
+		RK_ASSERT (levels->getDataType () == RData::StringVector);
+		QStringList new_levels = levels->stringVector ();
+		int levels_len = new_levels.size ();
+		RK_ASSERT (levels_len >= 1);
+		delete data->value_labels;
+		data->value_labels = new RObject::ValueLabels;
+		if ((levels_len == 1) && new_levels.at (0).isEmpty ()) {
+			// no levels
+		} else {
+			for (int i=0; i < levels_len; ++i) {
+				data->value_labels->insert (QString::number (i+1), new_levels.at (i));
+			}
+		}
+
+		// now set the data
+		RK_ASSERT (cdata->getDataLength () == (unsigned int) getLength ()); // not really a problem due to the line below, I'd still like to know if / when this happens.
+		extendToLength (cdata->getDataLength ());
+		if (cdata->getDataType () == RData::StringVector) {
+			setCharacterFromR (0, getLength () - 1, cdata->stringVector ());
+		} else if (cdata->getDataType () == RData::RealVector) {
+			setNumericFromR (0, getLength () - 1, cdata->realVector ());
+		} else if (cdata->getDataType () == RData::IntVector) {
+			RData::IntStorage int_data = cdata->intVector ();
+			unsigned int len = getLength ();
+			QVector<double> dd;
+			dd.reserve (len);
+			for (unsigned int i = 0; i < len; ++i) {
+				if (RInterface::isNaInt (int_data.at (i))) dd.append (NAN);
+				else dd.append ((double) int_data.at (i));
+			}
+			setNumericFromR (0, getLength () - 1, dd);
+		}
+
+		// now set the invalid fields (only if they are still NAs in the R data)
+		data->invalid_fields.clear ();
+		if (invalids->getDataLength () <= 1) {
+			// no invalids
+		} else {
+			RK_ASSERT (invalids->getDataType () == RData::StringVector);
+			QStringList invalids_list = invalids->stringVector ();
+			int invalids_length = invalids_list.size ();
+			RK_ASSERT ((invalids_length % 2) == 0);
+			int invalids_count = invalids_length / 2;
+			for (int i=0; i < invalids_count; ++i) {
+				int row = invalids_list.at (i).toInt () - 1;
+				if (data->cell_states[row] & RKVarEditData::NA) {	// NOTE: Do *not* use setText(), here. It tries too hard to set a valid value.
+					data->invalid_fields.insert (row, invalids_list.at (invalids_count + i));
+					data->cell_states[row] = RKVarEditData::Invalid;
+				}
+			}
+		}
+		data->previously_valid = data->invalid_fields.isEmpty ();
+		data->formatting_options = parseFormattingOptionsString (getMetaProperty ("format"));
+
+		ChangeSet *set = new ChangeSet (0, getLength (), true);
+		RKModificationTracker::instance()->objectDataChanged (this, set);
+		RKModificationTracker::instance()->objectMetaChanged (this);
+		type -= (type & NeedDataUpdate);
+		discardUnsyncedChanges ();
+		lockSyncing (false);
+	});
+	RInterface::issueCommand(c, chain);
 }
 
 void RKVariable::lockSyncing (bool lock) {
diff --git a/rkward/core/rkvariable.h b/rkward/core/rkvariable.h
index d0e49b03..62e7b195 100644
--- a/rkward/core/rkvariable.h
+++ b/rkward/core/rkvariable.h
@@ -35,8 +35,6 @@ public:
 
 /** reimplemented from RObject to also store value labels/factor levels (and in the future probably further info) */
 	void writeMetaData (RCommandChain *chain) override;
-friend class RContainerObject;
-	void rCommandDone (RCommand *command) override;
 public:
 ////////////// BEGIN: data handling ////////////////////////
 /** the Status enum is used for both keeping track of the entire row and individual cells. For single cells the meaning should be obvious. The entire row
diff --git a/rkward/core/robject.cpp b/rkward/core/robject.cpp
index 8cd591c2..0542b084 100644
--- a/rkward/core/robject.cpp
+++ b/rkward/core/robject.cpp
@@ -8,8 +8,8 @@ SPDX-License-Identifier: GPL-2.0-or-later
 
 #include "robject.h"
 
-#include <qregexp.h>
 #include <KLocalizedString>
+#include <KMessageBox>
 
 #include "../rbackend/rkrinterface.h"
 #include "../rbackend/rkrbackendprotocol_shared.h"
@@ -21,6 +21,7 @@ SPDX-License-Identifier: GPL-2.0-or-later
 #include "rfunctionobject.h"
 #include "rkmodificationtracker.h"
 #include "rkrownames.h"
+#include "../settings/rksettingsmoduleobjectbrowser.h"
 
 #include "../debug.h"
 
@@ -28,6 +29,40 @@ namespace RObjectPrivate {
 	QVector<qint32> dim_null (1, 0);
 }
 
+/** Proxy to guard against the unlikely - but possible - case that an RObject is deleted while it still has RCommands outstanding.
+ *  This is needed, because RObject is not QObject dereived. */
+class RObjectLifeTimeGuard {
+public:
+	RObjectLifeTimeGuard(RObject *object) : command_count(0), object(object) {
+		object->guard = this;
+	};
+	~RObjectLifeTimeGuard() {
+		if (object) {
+			object->guard = nullptr;
+		}
+	}
+	void addCommandFinishedCallback(RCommand *command, std::function<void(RCommand*)> callback) {
+		++command_count;
+		QObject::connect(command->notifier(), &RCommandNotifier::commandFinished, [this, callback](RCommand* command) {
+			if (object) {
+				callback(command);
+			}
+			if (--command_count <= 0) delete this;
+		});
+	}
+private:
+	int command_count;
+friend class RObject;
+	RObject *object;
+};
+
+void RObject::whenCommandFinished(RCommand* command, std::function<void (RCommand *)> callback) {
+	if (!guard) {
+		guard = new RObjectLifeTimeGuard(this);
+	}
+	guard->addCommandFinishedCallback(command, callback);
+}
+
 // static
 QHash<const RObject*, RObject::PseudoObjectType> RObject::pseudo_object_types;
 QHash<const RObject*, RSlotsPseudoObject*> RObject::slots_objects;
@@ -43,12 +78,16 @@ RObject::RObject (RObject *parent, const QString &name) {
 	meta_map = 0;
 	contained_objects = 0;
 	dimensions = RObjectPrivate::dim_null;	// safe initialization
+	guard = nullptr;
 }
 
 RObject::~RObject () {
 	RK_TRACE (OBJECTS);
 
-	cancelOutstandingCommands ();
+	if (guard) {
+		RK_DEBUG(OBJECTS, DL_INFO, "object deleted while still waiting for command results");
+		guard->object = nullptr;
+	}
 	if (hasPseudoObject (SlotsObject)) delete slots_objects.take (this);
 	if (hasPseudoObject (NamespaceObject)) delete namespace_objects.take (this);
 	if (hasPseudoObject (RowNamesObject)) delete rownames_objects.take (this);
@@ -229,18 +268,39 @@ void RObject::writeMetaData (RCommandChain *chain) {
 void RObject::updateFromR (RCommandChain *chain) {
 	RK_TRACE (OBJECTS);
 
-	RCommand *command;
+	QString commandstring;
 	if (parentObject () == RObjectList::getGlobalEnv ()) {
-#ifdef __GNUC__
-#	warning TODO: find a generic solution
-#endif
 // We handle objects directly in .GlobalEnv differently. That's to avoid forcing promises, when addressing the object directly. In the long run, .rk.get.structure should be reworked to simply not need the value-argument in any case.
-		 command = new RCommand (".rk.get.structure.global (" + rQuote (getShortName ()) + ')', RCommand::App | RCommand::Sync | RCommand::GetStructuredData, QString (), this, ROBJECT_UDPATE_STRUCTURE_COMMAND);
+		commandstring = ".rk.get.structure.global (" + rQuote (getShortName ()) + ')';
+	} else if (isType(Environment)) {
+		REnvironmentObject *env = static_cast<REnvironmentObject*>(this);
+		if (isType(PackageEnv) && RKSettingsModuleObjectBrowser::isPackageBlacklisted(env->packageName())) {
+			KMessageBox::information (0, i18n ("The package '%1' (probably you just loaded it) is currently blacklisted for retrieving structure information. Practically this means, the objects in this package will not appear in the object browser, and there will be no object name completion or function argument hinting for objects in this package.\nPackages will typically be blacklisted, if they contain huge amount of data, that would take too long to load. To unlist the package, visit Settings->Configure RKWard->Workspace.", env->packageName()), i18n("Package blacklisted"), "packageblacklist" + env->packageName());
+			return;
+		}
+		commandstring = ".rk.get.structure (" + getFullName(DefaultObjectNameOptions) + ", " + rQuote(getShortName());
+		if (isType(GlobalEnv)) commandstring += ", envlevel=-1";  // in the .GlobalEnv recurse one more level
+		if (isType(PackageEnv)) commandstring += ", namespacename=" + rQuote(env->packageName());
+		commandstring += ')';
 	} else {
 // This is the less common branch, but we do call .rk.get.structure on sub-object, e.g. when fetching more levels in the Workspace Browser, or when calling rk.sync(), explicitly
-		command = new RCommand (".rk.get.structure (" + getFullName () + ", " + rQuote (getShortName ()) + ')', RCommand::App | RCommand::Sync | RCommand::GetStructuredData, QString (), this, ROBJECT_UDPATE_STRUCTURE_COMMAND);
+		commandstring = ".rk.get.structure (" + getFullName () + ", " + rQuote (getShortName ()) + ')';
 	}
-	RInterface::issueCommand (command, chain);
+	RCommand *command = new RCommand(commandstring, RCommand::App | RCommand::Sync | RCommand::GetStructuredData);
+	whenCommandFinished(command, [this](RCommand* command) {
+		if (command->failed ()) {
+			RK_DEBUG (OBJECTS, DL_INFO, "command failed while trying to update object '%s'. No longer present?", getShortName ().toLatin1 ().data ());
+			// this may happen, if the object has been removed in the workspace in between
+			RKModificationTracker::instance()->removeObject (this, 0, true);
+			return;
+		}
+		if (parent && parent->isContainer()) {
+			static_cast<RContainerObject*>(parent)->updateChildStructure(this, command);		// this may result in a delete, so nothing after this!
+		} else {
+			updateStructure(command);		// no (container) parent can happen for RObjectList and pseudo objects
+		}
+	});
+	RInterface::issueCommand(command, chain);
 
 	type |= Updating;	// will be cleared, implicitly, when the new structure gets set
 }
@@ -264,24 +324,6 @@ void RObject::fetchMoreIfNeeded (int levels) {
 	}
 }
 
-void RObject::rCommandDone (RCommand *command) {
-	RK_TRACE (OBJECTS);
-
-	if (command->getFlags () == ROBJECT_UDPATE_STRUCTURE_COMMAND) {
-		if (command->failed ()) {
-			RK_DEBUG (OBJECTS, DL_INFO, "command failed while trying to update object '%s'. No longer present?", getShortName ().toLatin1 ().data ());
-			// this may happen, if the object has been removed in the workspace in between
-			RKModificationTracker::instance()->removeObject (this, 0, true);
-			return;
-		}
-		if (parent && parent->isContainer ()) static_cast<RContainerObject*> (parent)->updateChildStructure (this, command);		// this may result in a delete, so nothing after this!
-		else updateStructure (command);		// no (container) parent can happen for RObjectList and pseudo objects
-		return;
-	} else {
-		RK_ASSERT (false);
-	}
-}
-
 bool RObject::updateStructure (RData *new_data) {
 	RK_TRACE (OBJECTS);
 	if (new_data->getDataLength () == 0) { // can happen, if the object no longer exists
@@ -293,10 +335,7 @@ bool RObject::updateStructure (RData *new_data) {
 
 	if (!canAccommodateStructure (new_data)) return false;
 
-	if (isPending ()) {
-		type -= Pending;
-		return true;	// Do not update any info for pending objects
-	}
+	if (isPending ()) type -= Pending;
 
 	bool properties_change = false;
 
diff --git a/rkward/core/robject.h b/rkward/core/robject.h
index 12135a80..c1898492 100644
--- a/rkward/core/robject.h
+++ b/rkward/core/robject.h
@@ -12,8 +12,7 @@ SPDX-License-Identifier: GPL-2.0-or-later
 #include <qstring.h>
 #include <qmap.h>
 #include <QHash>
-
-#include "../rbackend/rcommandreceiver.h"
+#include <QVector>
 
 class RSlotsPseudoObject;
 class REnvironmentObject;
@@ -22,8 +21,8 @@ class RKRowNames;
 class RCommandChain;
 class RKEditor;
 class RData;
-
-#define ROBJECT_UDPATE_STRUCTURE_COMMAND 1
+class RObjectLifeTimeGuard;
+class RCommand;
 
 /**
 Base class for representations of objects in the R-workspace. RObject is never used directly (contains pure virtual functions).
@@ -31,7 +30,7 @@ Base class for representations of objects in the R-workspace. RObject is never u
 @author Thomas Friedrichsmeier
 */
 
-class RObject : public RCommandReceiver {
+class RObject {
 public:
 	RObject (RObject *parent, const QString &name);
 	virtual ~RObject ();
@@ -297,7 +296,8 @@ friend class RKModificationTracker;
 /** Notify the object that a model no longer needs its data. If there have been as many endEdit() as beginEdit() calls, the object should discard its data storage. The default implementation does nothing (raises an assert). */
 	virtual void endEdit ();
 
-	void rCommandDone (RCommand *command) override;
+/** wrapper around RCommand::whenFinishe(), needed because RObject is not a Qbject subclass */
+	void whenCommandFinished(RCommand *command, std::function<void(RCommand*)> callback);
 
 /* Storage hashes for special objects which are held by some but not all objects, and thus should not have a pointer
  * in the class declaration. Some apply only to specific RObject types, but moving storage to the relevant classes, would make it more
@@ -305,6 +305,8 @@ friend class RKModificationTracker;
 	static QHash<const RObject*, RSlotsPseudoObject*> slots_objects;
 	static QHash<const RObject*, REnvironmentObject*> namespace_objects;
 	static QHash<const RObject*, RKRowNames*> rownames_objects;
+friend class RObjectLifeTimeGuard;
+	RObjectLifeTimeGuard *guard;
 
 friend class RSlotsPseudoObject;
 friend class RKPackageNamespaceObject;
diff --git a/rkward/core/robjectlist.cpp b/rkward/core/robjectlist.cpp
index afdad12b..4562a628 100644
--- a/rkward/core/robjectlist.cpp
+++ b/rkward/core/robjectlist.cpp
@@ -8,9 +8,6 @@ SPDX-License-Identifier: GPL-2.0-or-later
 
 #define UPDATE_DELAY_INTERVAL 500
 
-#define ROBJECTLIST_UPDATE_ENVIRONMENTS_COMMAND 1
-#define ROBJECTLIST_UPDATE_COMPLETE_COMMAND 2
-
 #include <qtimer.h>
 #include <qstringlist.h>
 
@@ -118,7 +115,20 @@ void RObjectList::updateFromR (RCommandChain *chain) {
 	emit updateStarted();
 	update_chain = RInterface::startChain (chain);
 
-	RCommand *command = new RCommand ("list (search (), loadedNamespaces ())", RCommand::App | RCommand::Sync | RCommand::GetStructuredData, QString (), this, ROBJECTLIST_UPDATE_ENVIRONMENTS_COMMAND);
+	RCommand *command = new RCommand("list (search (), loadedNamespaces ())", RCommand::App | RCommand::Sync | RCommand::GetStructuredData);
+	whenCommandFinished(command, [this](RCommand* command) {
+		RK_ASSERT (command->getDataType () == RData::StructureVector);
+		const RData::RDataStorage & data = command->structureVector ();
+		RK_ASSERT (data.size () == 2);
+
+		QStringList new_environments = data[0]->stringVector ();
+		RK_ASSERT (new_environments.size () >= 2);
+
+		updateEnvironments (new_environments, true);
+		updateNamespaces (data[1]->stringVector ());
+
+		makeUpdateCompleteCallback();
+	});
 	RInterface::issueCommand (command, update_chain);
 }
 
@@ -139,34 +149,21 @@ void RObjectList::updateFromR (RCommandChain *chain, const QStringList &current_
 	updateEnvironments (current_searchpath, false);
 	updateNamespaces (current_namespaces);
 
-	RInterface::issueCommand (QString (), RCommand::App | RCommand::Sync | RCommand::EmptyCommand, QString (), this, ROBJECTLIST_UPDATE_COMPLETE_COMMAND, update_chain);
+	makeUpdateCompleteCallback();
 }
 
-void RObjectList::rCommandDone (RCommand *command) {
-	RK_TRACE (OBJECTS);
-
-	if (command->getFlags () == ROBJECTLIST_UPDATE_ENVIRONMENTS_COMMAND) {
-		RK_ASSERT (command->getDataType () == RData::StructureVector);
-		const RData::RDataStorage & data = command->structureVector ();
-		RK_ASSERT (data.size () == 2);
-		
-		QStringList new_environments = data[0]->stringVector ();
-		RK_ASSERT (new_environments.size () >= 2);
-
-		updateEnvironments (new_environments, true);
-		updateNamespaces (data[1]->stringVector ());
-
-		RInterface::issueCommand (QString (), RCommand::App | RCommand::Sync | RCommand::EmptyCommand, QString (), this, ROBJECTLIST_UPDATE_COMPLETE_COMMAND, update_chain);
-	} else if (command->getFlags () == ROBJECTLIST_UPDATE_COMPLETE_COMMAND) {
+void RObjectList::makeUpdateCompleteCallback() {
+	RK_TRACE(OBJECTS);
+	RCommand* command = new RCommand(QString(), RCommand::App | RCommand::Sync | RCommand::EmptyCommand);
+	whenCommandFinished(command, [this](RCommand*) {
 		RK_ASSERT (update_chain);
 		RInterface::closeChain (update_chain);
 		update_chain = 0;
-	
+
 		RK_DEBUG (OBJECTS, DL_DEBUG, "object list update complete");
 		emit updateComplete();
-	} else {
-		RK_ASSERT (false);
-	}
+	});
+	RInterface::issueCommand(command, update_chain);
 }
 
 void RObjectList::updateEnvironments (const QStringList &_env_names, bool force_globalenv_update) {
diff --git a/rkward/core/robjectlist.h b/rkward/core/robjectlist.h
index 24253af0..6e464820 100644
--- a/rkward/core/robjectlist.h
+++ b/rkward/core/robjectlist.h
@@ -74,7 +74,6 @@ protected:
 /// reimplemented from RContainerObject to emit a change signal
 	void objectsChanged ();
 	bool updateStructure (RData *new_data) override;
-	void rCommandDone (RCommand *command) override;
 	void updateEnvironments (const QStringList &env_names, bool force_globalenv_update);
 	void updateNamespaces (const QStringList &namespace_names);
 private:
@@ -86,6 +85,7 @@ private:
 	RKOrphanNamespacesObject *orphan_namespaces;
 
 	REnvironmentObject *createTopLevelEnvironment (const QString &name);
+	void makeUpdateCompleteCallback();
 
 	REnvironmentObject *globalenv;
 	static RObjectList *object_list;



More information about the rkward-tracker mailing list