[rkward-cvs] SF.net SVN: rkward:[2695] trunk/rkward

tfry at users.sourceforge.net tfry at users.sourceforge.net
Tue Oct 6 16:12:03 UTC 2009


Revision: 2695
          http://rkward.svn.sourceforge.net/rkward/?rev=2695&view=rev
Author:   tfry
Date:     2009-10-06 16:12:03 +0000 (Tue, 06 Oct 2009)

Log Message:
-----------
Properly fix detection of new output. This is much cleaner and takes us another step closer to having multiple output windows.

Modified Paths:
--------------
    trunk/rkward/ChangeLog
    trunk/rkward/rkward/plugin/rkstandardcomponentgui.cpp
    trunk/rkward/rkward/rbackend/rinterface.cpp
    trunk/rkward/rkward/rbackend/rthread.cpp
    trunk/rkward/rkward/rbackend/rthread.h
    trunk/rkward/rkward/windows/rkhtmlwindow.cpp
    trunk/rkward/rkward/windows/rkhtmlwindow.h
    trunk/rkward/rkward/windows/rkworkplace.cpp
    trunk/rkward/rkward/windows/rkworkplace.h
    trunk/rkward/tests/test_framework.R

Modified: trunk/rkward/ChangeLog
===================================================================
--- trunk/rkward/ChangeLog	2009-10-06 16:09:48 UTC (rev 2694)
+++ trunk/rkward/ChangeLog	2009-10-06 16:12:03 UTC (rev 2695)
@@ -1,4 +1,4 @@
-	  TODO: We need to fix the output detection properly, after all. Otherwise, if the plugin produces an error, no re-run link is shown.
+	  TODO: Test new output detection on windows
 - Add support for the dynamic help server introduced in R 2.10.0	TODO: backport (r2672, r2677)
 - Assorted minor fixes and improvments to several plugins
 - Fix deadlock while handling some Tcl events		TODO: much more (stress-)testing, backport? (r2649)

Modified: trunk/rkward/rkward/plugin/rkstandardcomponentgui.cpp
===================================================================
--- trunk/rkward/rkward/plugin/rkstandardcomponentgui.cpp	2009-10-06 16:09:48 UTC (rev 2694)
+++ trunk/rkward/rkward/plugin/rkstandardcomponentgui.cpp	2009-10-06 16:12:03 UTC (rev 2695)
@@ -160,8 +160,11 @@
 	command.append (code_property->calculate ());
 	command.append (code_property->printout ());
 	command.append ("})\n");
+	RKGlobals::rInterface ()->issueCommand (new RCommand (command, RCommand::Plugin | RCommand::DirectToOutput | RCommand::ObjectListUpdate), component->commandChain ());
 
 	// re-run link
+	// This should be run in a separate command, in case the above command bails out with an error. Even in that case, the re-run link should be printed.
+	command.clear ();
 	RKComponentHandle *handle = component->getHandle ();
 	if (handle->isAccessible ()) {
 		command.append (".rk.rerun.plugin.link(plugin=\"" + RKComponentMap::getComponentId (handle) + "\", settings=\"" + RKCommonFunctions::escape (component->serializeState ()) + "\", label=\"" + i18n ("Run again") + "\")\n");

Modified: trunk/rkward/rkward/rbackend/rinterface.cpp
===================================================================
--- trunk/rkward/rkward/rbackend/rinterface.cpp	2009-10-06 16:09:48 UTC (rev 2694)
+++ trunk/rkward/rkward/rbackend/rinterface.cpp	2009-10-06 16:12:03 UTC (rev 2695)
@@ -32,6 +32,7 @@
 #include "../windows/rcontrolwindow.h"
 #include "../windows/rkworkplace.h"
 #include "../windows/rkcommandlog.h"
+#include "../windows/rkhtmlwindow.h"
 #include "../plugin/rkcomponentmap.h"
 #include "../misc/rkcommonfunctions.h"
 
@@ -324,10 +325,10 @@
 		} else {
 			issueCommand (".rk.set.reply (\"Too few arguments in call to get.tempfile.name.\")", RCommand::App | RCommand::Sync, QString::null, 0, 0, request->in_chain);
 		}
-	} else if (call == "refreshOutput") {
-		RK_ASSERT (request->call.count () == 2); //but we don't use the second parameter, yet
+	} else if (call == "set.output.file") {
+		RK_ASSERT (request->call.count () == 2);
 
-		RKWorkplace::mainWorkplace ()->refreshOutputWindow ();
+		RKOutputWindowManager::self ()->setCurrentOutputPath (request->call[1]);
 	} else if (call == "sync") {
 		RK_ASSERT (request->call.count () >= 2);
 

Modified: trunk/rkward/rkward/rbackend/rthread.cpp
===================================================================
--- trunk/rkward/rkward/rbackend/rthread.cpp	2009-10-06 16:09:48 UTC (rev 2694)
+++ trunk/rkward/rkward/rbackend/rthread.cpp	2009-10-06 16:12:03 UTC (rev 2695)
@@ -233,11 +233,6 @@
 				RK_ASSERT (!error);
 			}
 		}
-		if (!(command->type () & RCommand::Sync)) {
-			MUTEX_UNLOCK;
-			checkNotifyOutputTouched ();
-			MUTEX_LOCK;
-		}
 	
 		if (error) {
 			RK_DO (qDebug ("- error message was: '%s'", command->error ().toLatin1 ().data ()), RBACKEND, dl);
@@ -388,20 +383,6 @@
 	MUTEX_UNLOCK;
 }
 
-void RThread::checkNotifyOutputTouched () {
-	RK_TRACE (RBACKEND);
-// TODO: instead of this, the output window(s) should simply watch the file for changes using
-// KDirWatch.
-
-	QFileInfo info (active_output_file);
-	if (info.exists ()) {
-		if ((!output_last_modified.isValid ()) || (output_last_modified < info.lastModified ())) {
-			output_last_modified = info.lastModified ();
-			handleSubstackCall (QStringList () << "refreshOutput" << active_output_file);
-		}
-	}
-}
-
 void RThread::handleSubstackCall (QStringList &call) {
 	RK_TRACE (RBACKEND);
 
@@ -412,11 +393,6 @@
 				if (!changed_symbol_names.contains (call[1])) changed_symbol_names.append (call[1]);
 			}
 			return;
-		} else if (call[0] == "set.output.file") {
-			checkNotifyOutputTouched ();
-			active_output_file = call[1];
-			output_last_modified = QFileInfo (active_output_file).lastModified ();
-			return;
 		}
 	}
 

Modified: trunk/rkward/rkward/rbackend/rthread.h
===================================================================
--- trunk/rkward/rkward/rbackend/rthread.h	2009-10-06 16:09:48 UTC (rev 2694)
+++ trunk/rkward/rkward/rbackend/rthread.h	2009-10-06 16:12:03 UTC (rev 2695)
@@ -196,7 +196,6 @@
 /** This is the function in which an RCommand actually gets processed. Basically it passes the command to REmbedInteranl::runCommandInternal () and sends RInterface some events about what is currently happening. */
 	void doCommand (RCommand *command);
 	void notifyCommandDone (RCommand *command);
-	void checkNotifyOutputTouched ();
 /** thread is locked. No new commands will be executed. @see LockType @see lock @see unlock */
 	int locked;
 /** thread is killed. Should exit as soon as possible. @see kill */
@@ -213,9 +212,6 @@
 /** check wether the object list / global environment / individual symbols have changed, and updates them, if needed */
 	void checkObjectUpdatesNeeded (bool check_list);
 	QList<RCommand*> all_current_commands;
-
-	QString active_output_file;
-	QDateTime output_last_modified;
 };
 
 #endif

Modified: trunk/rkward/rkward/windows/rkhtmlwindow.cpp
===================================================================
--- trunk/rkward/rkward/windows/rkhtmlwindow.cpp	2009-10-06 16:09:48 UTC (rev 2694)
+++ trunk/rkward/rkward/windows/rkhtmlwindow.cpp	2009-10-06 16:12:03 UTC (rev 2695)
@@ -41,6 +41,7 @@
 #include "../rkconsole.h"
 #include "../settings/rksettingsmodulegeneral.h"
 #include "../settings/rksettingsmoduler.h"
+#include "../settings/rksettingsmoduleoutput.h"
 #include "../misc/rkcommonfunctions.h"
 #include "../misc/rkstandardactions.h"
 #include "../misc/rkstandardicons.h"
@@ -51,9 +52,6 @@
 #include "../windows/rkworkplaceview.h"
 #include "../debug.h"
 
-//static 
-RKHTMLWindow* RKHTMLWindow::current_output = 0;
-
 RKHTMLWindow::RKHTMLWindow (QWidget *parent, WindowMode mode) : RKMDIWindow (parent, RKMDIWindow::HelpWindow) {
 	RK_TRACE (APP);
 	scroll_position=-1;
@@ -88,11 +86,6 @@
 RKHTMLWindow::~RKHTMLWindow () {
 	RK_TRACE (APP);
 
-	if (this == current_output) {
-		RK_ASSERT (window_mode == HTMLOutputWindow);
-		current_output = 0;
-	}
-
 	delete khtmlpart;
 }
 
@@ -243,7 +236,6 @@
 				fixed_url.setPath (url.path ());
 				if (url.hasQuery ()) fixed_url.setQuery (url.query ());
 				if (url.hasFragment ()) fixed_url.setFragment (url.fragment ());
-qDebug ("%s -> %s", qPrintable (url.url ()), qPrintable (fixed_url.url ()));
 				ok = openURL (fixed_url);
 			}
 		
@@ -268,6 +260,8 @@
 			RK_ASSERT (false);
 			return false;
 		}
+		current_url = url;	// needs to be set before registering
+		RKOutputWindowManager::self ()->registerWindow (this);
 	} else {
 		if (!(url.isLocalFile ())) {
 			if (window_mode == HTMLHelpWindow) {
@@ -356,38 +350,6 @@
 	}
 }
 
-//static
-RKHTMLWindow* RKHTMLWindow::getCurrentOutput () {
-	RK_TRACE (APP);
-	
-	if (!current_output) {
-		current_output = new RKHTMLWindow (RKWorkplace::mainWorkplace ()->view (), HTMLOutputWindow);
-
-		KUrl url (RKSettingsModuleGeneral::filesPath () + "/rk_out.html");
-		current_output->openURL (url);
-	}
-
-	return current_output;
-}
-
-//static
-RKHTMLWindow* RKHTMLWindow::refreshOutput (bool show, bool raise) {
-	RK_TRACE (APP);
-
-	if (current_output) {
-		if (raise) {
-			current_output->activate ();
-		}
-		current_output->refresh ();
-	} else {
-		if (show) {
-			// getCurrentOutput creates an output window
-			getCurrentOutput ();
-		}
-	}
-	return current_output;
-}
-
 void RKHTMLWindow::useMode (WindowMode new_mode) {
 	RK_TRACE (APP);
 
@@ -729,4 +691,129 @@
 	return (ret);
 }
 
+
+/////////////////////////////////////
+/////////////////////////////////////
+
+
+// static
+RKOutputWindowManager* RKOutputWindowManager::_self = 0;
+
+RKOutputWindowManager* RKOutputWindowManager::self () {
+	if (!_self) {
+		RK_TRACE (APP);
+
+		_self = new RKOutputWindowManager ();
+	}
+	return _self;
+}
+
+RKOutputWindowManager::RKOutputWindowManager () : QObject () {
+	RK_TRACE (APP);
+
+	file_watcher = new KDirWatch (this);
+	connect (file_watcher, SIGNAL (dirty(const QString&)), this, SLOT (fileChanged(const QString&)));
+	connect (file_watcher, SIGNAL (created(const QString&)), this, SLOT (fileChanged(const QString&)));
+}
+
+RKOutputWindowManager::~RKOutputWindowManager () {
+	RK_TRACE (APP);
+
+	file_watcher->removeFile (current_default_path);
+	delete (file_watcher);
+}
+
+void RKOutputWindowManager::registerWindow (RKHTMLWindow *window) {
+	RK_TRACE (APP);
+
+	RK_ASSERT (window->mode () == RKHTMLWindow::HTMLOutputWindow);
+	KUrl url = window->url ();
+
+	if (!url.isLocalFile ()) {
+		RK_ASSERT (false);		// should not happen right now, but might be an ok condition in the future. We can't monitor non-local files, though.
+		return;
+	}
+
+	url.cleanPath ();
+	QString file = url.toLocalFile ();
+	if (!windows.contains (file, window)) {
+		if (!windows.contains (file)) {
+			if (file != current_default_path) file_watcher->addFile (file);
+		}
+	
+		windows.insertMulti (file, window);
+		connect (window, SIGNAL (destroyed(QObject*)), this, SLOT (windowDestroyed(QObject*)));
+	} else {
+		RK_ASSERT (false);
+	}
+}
+
+void RKOutputWindowManager::setCurrentOutputPath (const QString &_path) {
+	RK_TRACE (APP);
+
+	KUrl url = KUrl::fromLocalFile (_path);
+	url.cleanPath ();
+	QString path = url.toLocalFile ();
+
+	if (path == current_default_path) return;
+
+	if (!windows.contains (path)) {
+		file_watcher->addFile (path);
+	}
+	if (!windows.contains (current_default_path)) {
+		file_watcher->removeFile (current_default_path);
+	}
+
+	current_default_path = path;
+}
+
+RKHTMLWindow* RKOutputWindowManager::getCurrentOutputWindow () {
+	RK_TRACE (APP);
+
+	RKHTMLWindow *current_output = windows.value (current_default_path);
+
+	if (!current_output) {
+		current_output = new RKHTMLWindow (RKWorkplace::mainWorkplace ()->view (), RKHTMLWindow::HTMLOutputWindow);
+
+		current_output->openURL (KUrl::fromLocalFile (current_default_path));
+
+		RK_ASSERT (current_output->url ().toLocalFile () == current_default_path);
+	}
+
+	return current_output;
+}
+
+void RKOutputWindowManager::fileChanged (const QString &path) {
+	RK_TRACE (APP);
+
+	RKHTMLWindow *w = 0;
+	QList<RKHTMLWindow *> window_list = windows.values (path);
+	for (int i = 0; i < window_list.size (); ++i) {
+		window_list[i]->refresh ();
+		w = window_list[i];
+	}
+
+	if (w) {
+		if (RKSettingsModuleOutput::autoRaise ()) w->activate ();
+	} else {
+		RK_ASSERT (path == current_default_path);
+		if (RKSettingsModuleOutput::autoShow ()) RKWorkplace::mainWorkplace ()->openOutputWindow (path);
+	}
+}
+
+void RKOutputWindowManager::windowDestroyed (QObject *window) {
+	RK_TRACE (APP);
+
+	// warning: Do not call any methods on the window. It is half-destroyed, already.
+	RKHTMLWindow *w = static_cast<RKHTMLWindow*> (window);
+
+	QString path = windows.key (w);
+	windows.remove (path, w);
+
+	// if there are no further windows for this file, stop listening
+	if ((path != current_default_path) && (!windows.contains (path))) {
+		file_watcher->removeFile (path);
+	}
+}
+
 #include "rkhtmlwindow.moc"

Modified: trunk/rkward/rkward/windows/rkhtmlwindow.h
===================================================================
--- trunk/rkward/rkward/windows/rkhtmlwindow.h	2009-10-06 16:09:48 UTC (rev 2694)
+++ trunk/rkward/rkward/windows/rkhtmlwindow.h	2009-10-06 16:12:03 UTC (rev 2695)
@@ -67,12 +67,7 @@
 	KUrl url ();
 	void doGotoAnchor (const QString &anchor_name);
 
-/** return a pointer to the current output. If there is no output window, one will be created (and shown) automatically */
-	static RKHTMLWindow* getCurrentOutput ();
-/** refresh output window.
- at param show Show the window, if not currently shown (this actually means: it is created if not currently existant)
- at param raise Raise the window (if currently shown, or show==true) */
-	static RKHTMLWindow* refreshOutput (bool show, bool raise);
+	WindowMode mode () { return window_mode; };
 public slots:
 /** this is used for browsing only. Use openURL instead, when calling from outside. */
 	void slotOpenUrl (const KUrl & url, const KParts::OpenUrlArguments &, const KParts::BrowserArguments &);
@@ -123,8 +118,6 @@
 
 	void fileDoesNotExistMessage ();
 
-	static RKHTMLWindow *current_output;
-
 	// for dealing with rkward://[page|component]-pages
 	bool renderRKHelp (const KUrl &url);
 	QString renderHelpFragment (QDomElement &fragment);
@@ -148,5 +141,32 @@
 	~RKHelpRenderer () {};
 };
 
+#include <QMultiHash>
 
+#include <kdirwatch.h>
+
+/** Takes care of showing / refreshing output windows as needed. */
+class RKOutputWindowManager : public QObject {
+Q_OBJECT
+public:
+	static RKOutputWindowManager *self ();
+
+	void registerWindow (RKHTMLWindow *window);
+/** R may produce output while no output window is active. This allows to set the file that should be monitored for such changes (called from within rk.set.html.output.file()). */
+	void setCurrentOutputPath (const QString &path);
+/** return a pointer to the current output. If there is no output window, one will be created (and shown) automatically */
+	RKHTMLWindow* getCurrentOutputWindow ();
+private:
+	RKOutputWindowManager ();
+	~RKOutputWindowManager ();
+	static RKOutputWindowManager *_self;
+
+	QString current_default_path;
+	KDirWatch *file_watcher;
+	QMultiHash<QString, RKHTMLWindow *> windows;
+private slots:
+	void fileChanged (const QString &path);
+	void windowDestroyed (QObject *window);
+};
+
 #endif

Modified: trunk/rkward/rkward/windows/rkworkplace.cpp
===================================================================
--- trunk/rkward/rkward/windows/rkworkplace.cpp	2009-10-06 16:09:48 UTC (rev 2694)
+++ trunk/rkward/rkward/windows/rkworkplace.cpp	2009-10-06 16:12:03 UTC (rev 2695)
@@ -39,7 +39,6 @@
 #include "../dataeditor/rkeditor.h"
 #include "../dataeditor/rkeditordataframe.h"
 #include "../robjectviewer.h"
-#include "../settings/rksettingsmoduleoutput.h"
 #include "../settings/rksettingsmodulegeneral.h"
 #include "../rbackend/rinterface.h"
 #include "../windows/rkwindowcatcher.h"
@@ -274,22 +273,14 @@
 void RKWorkplace::openOutputWindow (const KUrl &url) {
 	RK_TRACE (APP);
 
-	RKHTMLWindow::refreshOutput (true, true);
-	if (!windows.contains (RKHTMLWindow::getCurrentOutput ())) {
-		addWindow (RKHTMLWindow::getCurrentOutput ());
+	RKHTMLWindow *w = RKOutputWindowManager::self ()->getCurrentOutputWindow ();
+	if (!windows.contains (w)) {
+		addWindow (w);
+	} else {
+		w->activate ();
 	}
 }
 
-void RKWorkplace::refreshOutputWindow () {
-	RK_TRACE (APP);
-	RKHTMLWindow *window = RKHTMLWindow::refreshOutput (RKSettingsModuleOutput::autoShow (), RKSettingsModuleOutput::autoRaise ());
-	if (window) {
-		if (!windows.contains (window)) {
-			addWindow (window);
-		}
-	}
-}
-
 void RKWorkplace::newX11Window (WId window_to_embed, int device_number) {
 	RK_TRACE (APP);
 

Modified: trunk/rkward/rkward/windows/rkworkplace.h
===================================================================
--- trunk/rkward/rkward/windows/rkworkplace.h	2009-10-06 16:09:48 UTC (rev 2694)
+++ trunk/rkward/rkward/windows/rkworkplace.h	2009-10-06 16:12:03 UTC (rev 2695)
@@ -107,9 +107,6 @@
 /** Opens a new output window. Currently only a single output window will ever be created. Subsequent calls to the function will not create additional windows right now (but will raise / refresh the output window
 @param url currently ignored! */
 	void openOutputWindow (const KUrl &url=KUrl ());
-/** signal there was new output, show/raise/refresh the output window as appropriate.
-TODO: this should be obsoleted somehow */
-	void refreshOutputWindow ();
 
 	void newX11Window (WId window_to_embed, int device_number);
 	void newObjectViewer (RObject *object);

Modified: trunk/rkward/tests/test_framework.R
===================================================================
--- trunk/rkward/tests/test_framework.R	2009-10-06 16:09:48 UTC (rev 2694)
+++ trunk/rkward/tests/test_framework.R	2009-10-06 16:12:03 UTC (rev 2695)
@@ -229,6 +229,8 @@
 		for (i in 1:length (suite at postCalls)) try (suite at postCalls[[i]]())
 	}
 
+	rktest.resetEnvironment ()
+
 	result
 }
 
@@ -287,5 +289,17 @@
 	# Make sure i18n does not get in the way
 	invisible (Sys.setenv (LANGUAGE="C"))
 	if (.Platform$OS.type == "unix") invisible (Sys.setlocale ("LC_MESSAGES", "C"))
+
+	# This version of rk.set.output.html.file does not notify the frontend of the change. Without this, you'll get lots of output windows.
+	rk.set.output.html.file <<- function (x) {
+		stopifnot(is.character(x))
+		assign(".rk.output.html.file", x, as.environment("package:rkward"))
+	}
 }
 rktest.initializeEnvironment ()
+
+# counterpart to rktest.initializeEnvironment. Restores the most important settings
+rktest.resetEnvironment <- function () {
+	rm (list=c ("rk.set.output.html.file", "rk.get.tempfile.name", ".rk.make.hr"), envir=globalenv ())
+	.rk.rerun.plugin.link <<- .rk.rerun.plugin.link.replacement
+}


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.




More information about the rkward-tracker mailing list