[education/rkward/devel/workspace_output] /: Allow R-levels calls to support both subcommands, and a return value at the same time.
Thomas Friedrichsmeier
null at kde.org
Fri Nov 6 16:12:26 GMT 2020
Git commit 4340558fb4e3933bdbcdade55293442605e72b58 by Thomas Friedrichsmeier.
Committed on 06/11/2020 at 16:11.
Pushed by tfry into branch 'devel/workspace_output'.
Allow R-levels calls to support both subcommands, and a return value at the same time.
M +1 -1 ChangeLog
M +6 -2 rkward/rbackend/rkbackendtransmitter.cpp
M +23 -10 rkward/rbackend/rkrbackend.cpp
M +4 -3 rkward/rbackend/rkrbackend.h
M +3 -2 rkward/rbackend/rkrbackendprotocol_shared.cpp
M +14 -2 rkward/rbackend/rkrbackendprotocol_shared.h
M +6 -3 rkward/rbackend/rkrinterface.cpp
M +8 -0 rkward/rbackend/rktransmitter.cpp
https://invent.kde.org/education/rkward/commit/4340558fb4e3933bdbcdade55293442605e72b58
diff --git a/ChangeLog b/ChangeLog
index e77bc892..08e7d648 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,11 +1,11 @@
TODOS for output directories:
- - Fix R backend requests that may have both a return value *and* subcommands
- Make sure not to activate default output while quitting (abouttoquit is too late)
- save outputs to .rkworkplace and restore them
- automated tests
- UI
- remove explicit warnings
+- Internal: Allow R-levels calls to support both subcommands, and a return value at the same time
- 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())
diff --git a/rkward/rbackend/rkbackendtransmitter.cpp b/rkward/rbackend/rkbackendtransmitter.cpp
index dc11b1ca..3ce13503 100644
--- a/rkward/rbackend/rkbackendtransmitter.cpp
+++ b/rkward/rbackend/rkbackendtransmitter.cpp
@@ -88,9 +88,13 @@ void RKRBackendTransmitter::writeRequest (RBackendRequest *request) {
transmitRequest (request);
connection->flush ();
+ if (request->subcommandrequest) {
+ current_sync_requests.append(request->subcommandrequest);
+ RK_DEBUG(RBACKEND, DL_DEBUG, "Expecting replies for %d requests (added subrequest %p)", current_sync_requests.size(), request);
+ }
if (request->synchronous) {
- current_sync_requests.append (request);
- RK_DEBUG (RBACKEND, DL_DEBUG, "Expecting replies for %d requests (added %p)", current_sync_requests.size (), request);
+ current_sync_requests.append(request);
+ RK_DEBUG(RBACKEND, DL_DEBUG, "Expecting replies for %d requests (added %p)", current_sync_requests.size(), request);
} else {
delete request;
}
diff --git a/rkward/rbackend/rkrbackend.cpp b/rkward/rbackend/rkrbackend.cpp
index acb90c07..f27968ea 100644
--- a/rkward/rbackend/rkrbackend.cpp
+++ b/rkward/rbackend/rkrbackend.cpp
@@ -946,7 +946,7 @@ SEXP doSubstackCall (SEXP call) {
} */
- auto ret = RKRBackend::this_pointer->handleHistoricalSubstackRequest(list);
+ auto ret = RKRBackend::this_pointer->handleRequestWithSubcommands(list);
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));
@@ -1562,14 +1562,25 @@ void RKRBackend::commandFinished (bool check_object_updates_needed) {
}
}
-RCommandProxy* RKRBackend::handleRequest (RBackendRequest *request, bool mayHandleSubstack) {
+RCommandProxy* RKRBackend::handleRequest(RBackendRequest *request, bool mayHandleSubstack) {
RK_TRACE (RBACKEND);
RK_ASSERT (request);
- RKRBackendProtocolBackend::instance ()->sendRequest (request);
+ // Seed 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);
if ((!request->synchronous) && (!isKilled ())) {
- RK_ASSERT (mayHandleSubstack); // i.e. not called from fetchNextCommand
+ RK_ASSERT(mayHandleSubstack); // i.e. not called from fetchNextCommand
+ RK_ASSERT(!request->subcommandrequest);
return 0;
}
@@ -1628,13 +1639,15 @@ RCommandProxy* RKRBackend::fetchNextCommand () {
return (handleRequest (&req, false));
}
-GenericRRequestResult RKRBackend::handleHistoricalSubstackRequest (const QStringList &list) {
+GenericRRequestResult RKRBackend::handleRequestWithSubcommands(const QStringList &list) {
RK_TRACE (RBACKEND);
- RBackendRequest request (true, RBackendRequest::HistoricalSubstackRequest);
+ RBackendRequest request(true, RBackendRequest::GenericRequestWithSubcommands);
request.params["call"] = list;
request.command = current_command;
- handleRequest (&request);
+ request.subcommandrequest = new RBackendRequest(true, RBackendRequest::OtherRequest);
+ handleRequest(&request);
+ delete request.subcommandrequest;
return request.getResult();
}
@@ -1767,12 +1780,12 @@ void RKRBackend::checkObjectUpdatesNeeded (bool check_list) {
dummy = runDirectCommand ("loadedNamespaces ()\n", RCommand::GetStringVector);
call.append (dummy->stringVector ());
delete dummy;
- handleHistoricalSubstackRequest (call);
+ handleRequestWithSubcommands (call);
}
if (globalenv_update_needed) {
QStringList call = global_env_toplevel_names;
call.prepend ("syncglobal"); // should be faster than the reverse
- handleHistoricalSubstackRequest (call);
+ handleRequestWithSubcommands (call);
}
}
@@ -1784,7 +1797,7 @@ void RKRBackend::checkObjectUpdatesNeeded (bool check_list) {
if (!changed_symbol_names.isEmpty ()) {
QStringList call = changed_symbol_names;
call.prepend (QString ("sync")); // should be faster than reverse
- handleHistoricalSubstackRequest (call);
+ handleRequestWithSubcommands (call);
changed_symbol_names.clear ();
}
}
diff --git a/rkward/rbackend/rkrbackend.h b/rkward/rbackend/rkrbackend.h
index 3fedfe62..74780a95 100644
--- a/rkward/rbackend/rkrbackend.h
+++ b/rkward/rbackend/rkrbackend.h
@@ -104,10 +104,10 @@ public:
@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); };
+ 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);
+ GenericRRequestResult handleRequestWithSubcommands(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!) */
GenericRRequestResult handlePlainGenericRequest(const QStringList ¶meters, bool synchronous);
@@ -205,7 +205,8 @@ handleHistoricalSubstackRequest(). Exactly which requests get handled by which f
private:
void clearPendingInterrupt ();
protected:
- RCommandProxy* handleRequest (RBackendRequest *request, bool mayHandleSubstack);
+ 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.cpp b/rkward/rbackend/rkrbackendprotocol_shared.cpp
index e0ea6dd6..9406a616 100644
--- a/rkward/rbackend/rkrbackendprotocol_shared.cpp
+++ b/rkward/rbackend/rkrbackendprotocol_shared.cpp
@@ -43,8 +43,9 @@ RBackendRequest::RBackendRequest (bool synchronous, RCallbackType type) {
RBackendRequest::type = type;
id = ++_id;
done = false;
- command = 0;
- output = 0;
+ command = nullptr;
+ output = nullptr;
+ subcommandrequest = nullptr;
}
RBackendRequest::~RBackendRequest () {
diff --git a/rkward/rbackend/rkrbackendprotocol_shared.h b/rkward/rbackend/rkrbackendprotocol_shared.h
index ca159975..c2399960 100644
--- a/rkward/rbackend/rkrbackendprotocol_shared.h
+++ b/rkward/rbackend/rkrbackendprotocol_shared.h
@@ -2,7 +2,7 @@
rkrbackendprotocol - description
-------------------
begin : Thu Nov 04 2010
- copyright : (C) 2010-2018 by Thomas Friedrichsmeier
+ copyright : (C) 2010-2020 by Thomas Friedrichsmeier
email : thomas.friedrichsmeier at kdemail.net
***************************************************************************/
@@ -24,6 +24,17 @@
class RCommandProxy;
+/** Class to represent an "event" sent between backend and frontend. This encapuslates all communication sent between the two processes (actually the graphics device uses a separate channel, but that's not the point, here).
+ *
+ * - Most, but not all requests originate in the backend.
+ * - Some requests are "asynchronous" in the sense of fire and forget, i.e. the backend notifies the frontend of some condition, but does not wait for a reply.
+ * - Many requests are synchronous, in that they will block what the backend is currently doing, until the frontend posted the event back (with or without some kind of return value)). Notably, however, while waiting for a synchronous request to
+ * complete, the backend will still do event processing, which may include running further R commands (which may again involve sending requests to the frontend).
+ * - Some requests contain a "subcommandrequest". This allows the frontend to (optionally) send commands that will be executed _before_ the original/main request is completed.
+ * - All of this means that requests and their answers are not necessarily send/received in a sorted order.
+ * - In an attempt to streamline communication, the next command to process is generally sent as part of the answer to a request. This should probably be reconsidered.
+ * - The real mess, however, is that the whole mechanism has grown over time, and contains strange termms and ad-hoc additions. Parts of it should probably be re-designed from the ground up. Hopefully, these notes, help with that.
+ */
class RBackendRequest {
public:
enum RCallbackType {
@@ -37,7 +48,7 @@ public:
Started,
EvalRequest,
CallbackRequest,
- HistoricalSubstackRequest, // 10
+ GenericRequestWithSubcommands, // 10
PlainGenericRequest,
SetParamsFromBackend,
Debugger,
@@ -64,6 +75,7 @@ public:
return ret;
}
+ RBackendRequest *subcommandrequest;
/** Should this request be handled synchronously? False by default. */
bool synchronous;
/** For synchronous requests, only: The frontend-thread will set this to true (using completed()), once the request has been "completed". Important: The backend thread MUST NOT touch a request after it has been sent, and before "done" has been set to true. */
diff --git a/rkward/rbackend/rkrinterface.cpp b/rkward/rbackend/rkrinterface.cpp
index 3b8ef364..2d0aa8d9 100644
--- a/rkward/rbackend/rkrinterface.cpp
+++ b/rkward/rbackend/rkrinterface.cpp
@@ -392,7 +392,7 @@ void RInterface::handleRequest (RBackendRequest* request) {
handleCommandOut (command);
}
tryNextCommand ();
- } else if (request->type == RBackendRequest::HistoricalSubstackRequest) {
+ } else if (request->type == RBackendRequest::GenericRequestWithSubcommands) {
RCommandProxy *cproxy = request->takeCommand();
RCommand *parent = 0;
for (int i = all_current_commands.size () - 1; i >= 0; --i) {
@@ -402,8 +402,11 @@ void RInterface::handleRequest (RBackendRequest* request) {
}
}
delete cproxy;
- command_requests.append (request);
- processHistoricalSubstackRequest (request->params["call"].toStringList (), parent, request);
+ RK_ASSERT(request->subcommandrequest);
+ command_requests.append(request->subcommandrequest);
+ request->subcommandrequest = nullptr; // it is now a separate request. Make sure we won't try to send it back as part of this one.
+ processHistoricalSubstackRequest(request->params["call"].toStringList(), parent, request);
+ RKRBackendProtocolFrontend::setRequestCompleted(request);
} else if (request->type == RBackendRequest::PlainGenericRequest) {
request->setResult(processPlainGenericRequest(request->params["call"].toStringList()));
RKRBackendProtocolFrontend::setRequestCompleted (request);
diff --git a/rkward/rbackend/rktransmitter.cpp b/rkward/rbackend/rktransmitter.cpp
index 538c2599..69b00d46 100644
--- a/rkward/rbackend/rktransmitter.cpp
+++ b/rkward/rbackend/rktransmitter.cpp
@@ -40,6 +40,12 @@ void RKRBackendSerializer::serialize (const RBackendRequest &request, QDataStrea
stream << false;
}
stream << request.params;
+ if (request.subcommandrequest) {
+ stream << true;
+ serialize(*(request.subcommandrequest), stream);
+ } else {
+ stream << false;
+ }
}
RBackendRequest *RKRBackendSerializer::unserialize (QDataStream &stream) {
@@ -63,6 +69,8 @@ RBackendRequest *RKRBackendSerializer::unserialize (QDataStream &stream) {
stream >> dummyb;
if (dummyb) request->output = unserializeOutput (stream);
stream >> request->params;
+ stream >> dummyb;
+ if (dummyb) request->subcommandrequest = unserialize(stream);
return request;
}
More information about the rkward-tracker
mailing list