[education/rkward/devel/workspace_output] /: Save outputs as archive files, instead of plain directories.

Thomas Friedrichsmeier null at kde.org
Sun Oct 25 09:55:16 GMT 2020


Git commit 7a08b0b909fe586177e9caaec01137d3f0d2a30d by Thomas Friedrichsmeier.
Committed on 25/10/2020 at 09:54.
Pushed by tfry into branch 'devel/workspace_output'.

Save outputs as archive files, instead of plain directories.

This simplifies the handling a lot.

M  +1    -1    CMakeLists.txt
M  +1    -1    rkward/misc/CMakeLists.txt
M  +66   -88   rkward/misc/rkoutputdirectory.cpp
M  +4    -4    rkward/misc/rkoutputdirectory.h
M  +5    -4    rkward/rbackend/rpackages/rkward/R/rk.output.R

https://invent.kde.org/education/rkward/commit/7a08b0b909fe586177e9caaec01137d3f0d2a30d

diff --git a/CMakeLists.txt b/CMakeLists.txt
index de535a56..c47e5be7 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -22,7 +22,7 @@ INCLUDE(ECMMarkNonGuiExecutable)
 INCLUDE(FeatureSummary)
 
 FIND_PACKAGE(Qt5 5.2 CONFIG REQUIRED COMPONENTS Widgets Core Xml Network Script PrintSupport)
-FIND_PACKAGE(KF5 5.2 REQUIRED COMPONENTS CoreAddons DocTools I18n XmlGui TextEditor WidgetsAddons Parts Config Notifications WindowSystem OPTIONAL_COMPONENTS Crash)
+FIND_PACKAGE(KF5 5.2 REQUIRED COMPONENTS CoreAddons DocTools I18n XmlGui TextEditor WidgetsAddons Parts Config Notifications WindowSystem Archive OPTIONAL_COMPONENTS Crash)
 IF(NOT NO_QT_WEBENGINE)
 	FIND_PACKAGE(Qt5 5.2 OPTIONAL_COMPONENTS WebEngineWidgets)
 	IF(NOT Qt5WebEngineWidgets_FOUND OR Qt5WebEngineWidgets_VERSION VERSION_LESS "5.12.0")
diff --git a/rkward/misc/CMakeLists.txt b/rkward/misc/CMakeLists.txt
index 1628026e..7863abbd 100644
--- a/rkward/misc/CMakeLists.txt
+++ b/rkward/misc/CMakeLists.txt
@@ -31,4 +31,4 @@ SET(misc_STAT_SRCS
    )
 
 ADD_LIBRARY(misc STATIC ${misc_STAT_SRCS})
-TARGET_LINK_LIBRARIES(misc Qt5::Widgets KF5::WidgetsAddons KF5::KIOWidgets Qt5::Xml KF5::ConfigCore KF5::Parts KF5::WindowSystem KF5::TextEditor)
+TARGET_LINK_LIBRARIES(misc Qt5::Widgets KF5::WidgetsAddons KF5::KIOWidgets Qt5::Xml KF5::ConfigCore KF5::Parts KF5::WindowSystem KF5::TextEditor KF5::Archive)
diff --git a/rkward/misc/rkoutputdirectory.cpp b/rkward/misc/rkoutputdirectory.cpp
index 0484477a..d357d0ba 100644
--- a/rkward/misc/rkoutputdirectory.cpp
+++ b/rkward/misc/rkoutputdirectory.cpp
@@ -63,29 +63,6 @@ QString hashDirectoryState(const QString& dir) {
 //	return QCryptographicHash::hash (list.toUtf8 (), QCryptographicHash::Md5);
 }
 
-bool copyDirRecursively(const QString& _source_dir, const QString& _dest_dir) {
-	RK_TRACE(APP);
-
-	QDir dest_dir(_dest_dir);
-	QDir source_dir(_source_dir);
-	if (dest_dir.mkpath(".")) return false;
-	if (!source_dir.exists()) return false;
-
-	bool ok = true;
-	QFileInfoList entries = source_dir.entryInfoList(QDir::AllEntries | QDir::NoDotAndDotDot);
-	for (int i = 0; i < entries.size (); ++i) {
-		const QFileInfo fi = entries[i];
-		if (fi.isDir()) {
-			ok = ok && copyDirRecursively(fi.absoluteFilePath(), dest_dir.absoluteFilePath(fi.fileName ()));
-		} else {
-			// NOTE: this does not overwrite existing target files, but in our use case, we're always writing into empty targets
-			ok = ok && QFile::copy(fi.absoluteFilePath(), dest_dir.absoluteFilePath(fi.fileName ()));
-		}
-	}
-
-	return ok;
-}
-
 QMap<QString, RKOutputDirectory*> RKOutputDirectory::outputs;
 
 RKOutputDirectory::RKOutputDirectory() : initialized(false), window(nullptr) {
@@ -108,19 +85,25 @@ RKOutputDirectory* RKOutputDirectory::getOutputBySaveUrl(const QString& _dest) {
 	QString dest = QFileInfo(_dest).canonicalFilePath();
 	auto it = outputs.constBegin();
 	while (it != outputs.constEnd()) {
-		if (it.value()->save_dir == dest) {
+		if (it.value()->save_filename == dest) {
 			return(it.value());
 		}
 	}
 	return nullptr;
 }
 
-GenericRRequestResult RKOutputDirectory::save(const QString& dest, RKOutputDirectory::OverwriteBehavior overwrite) {
+GenericRRequestResult RKOutputDirectory::save(const QString& _dest, RKOutputDirectory::OverwriteBehavior overwrite) {
 	RK_TRACE (APP);
+
+	QString dest = _dest;
+	if (dest.isEmpty()) {
+		dest = filename();  // might still be empty, in which case exportAs will ask for destination
+		if (!dest.isEmpty()) overwrite = Force;  // don't prompt when writing to same file
+	}
 	GenericRRequestResult res = exportAs(dest, overwrite);
 	if (!res.failed()) {
 		updateSavedHash();
-		save_dir = res.ret.toString(); // might by different from dest, notably, if dest was empty
+		save_filename = res.ret.toString();  // might by different from dest, notably, if dest was empty
 	}
 	return res;
 }
@@ -130,9 +113,9 @@ GenericRRequestResult RKOutputDirectory::exportAs (const QString& _dest, RKOutpu
 
 	QString dest = _dest;
 	if (dest.isEmpty()) {
-		QFileDialog dialog(RKWardMainWindow::getMain(), i18n("Specify directory where to export output"), save_dir);
-		dialog.setFileMode(QFileDialog::Directory);
-		dialog.setOption(QFileDialog::ShowDirsOnly, true);
+		QFileDialog dialog(RKWardMainWindow::getMain(), i18n("Select destination file name"), QFileInfo(save_filename).absolutePath());
+		dialog.setFileMode(QFileDialog::AnyFile);
+		dialog.setNameFilters(QStringList() << i18n("RKWard Output Files (*.rko)") << i18n("All Files (*)"));
 		dialog.setAcceptMode(QFileDialog::AcceptSave);
 		dialog.setOption(QFileDialog::DontConfirmOverwrite, true);  // custom handling below
 
@@ -143,66 +126,72 @@ GenericRRequestResult RKOutputDirectory::exportAs (const QString& _dest, RKOutpu
 		dest = QDir::cleanPath(dialog.selectedFiles().value(0));
 	}
 
-	// If destination already exists, rename it before copying, so we can restore the save in case of an error
-	QString tempname;
-
-	dest = QFileInfo(dest).canonicalFilePath();
 	bool exists = QFileInfo(dest).exists();
 	if (exists) {
-#warning TODO Check terminology ("output directory") once finalized
-		if (!isRKWwardOutputDirectory(dest)) {
-			return GenericRRequestResult::makeError(i18n("The directory %1 exists, but does not appear to be an RKWard output directory. Refusing to overwrite it.", dest));
-		}
-/*		if (isOutputDirectoryModified(dest)) {
-			return GenericRRequestResult::makeError(i18n("The output directory %1 has been modified by an external process. Refusing to overwrite it.", dest));
-		} */
 		if (overwrite == Ask) {
-			const QString warning = i18n("Are you sure you want to overwrite the existing directory '%1'? All current contents, <b>including subdirectories</b> will be lost.", dest);
-			KMessageBox::ButtonCode res = KMessageBox::warningContinueCancel(RKWardMainWindow::getMain(), warning, i18n("Overwrite Directory?"), KStandardGuiItem::overwrite(),
+			const QString warning = i18n("Are you sure you want to overwrite the existing file '%1'?", dest);
+			KMessageBox::ButtonCode res = KMessageBox::warningContinueCancel(RKWardMainWindow::getMain(), warning, i18n("Overwrite?"), KStandardGuiItem::overwrite(),
 												KStandardGuiItem::cancel(), QString(), KMessageBox::Options(KMessageBox::Notify | KMessageBox::Dangerous));
 			if (KMessageBox::Continue != res) return GenericRRequestResult::makeError(i18n("User cancelled"));
 		} else if (overwrite != Force) {
-			return GenericRRequestResult::makeError(i18n("Not overwriting existing output"));
+			return GenericRRequestResult::makeError(i18n("Not overwriting existing file"));
 		}
+	}
 
-		tempname = dest + '~';
-		while (QFileInfo(tempname).exists()) {
-			tempname.append('~');
-		}
-		if (!QDir().rename(dest, tempname)) {
-			return GenericRRequestResult::makeError(i18n("Failed to create temporary backup file %1.", tempname));
-		}
+	return exportZipInternal(dest);
+}
+
+#include <KZip>
+GenericRRequestResult RKOutputDirectory::exportZipInternal(const QString &dest) {
+	RK_TRACE (APP);
+
+	// write to a temporary location, first, then - if successful - copy to final destination
+	QString tempname = dest + '~';
+	while (QFileInfo(tempname).exists()) {
+		tempname.append('~');
 	}
 
-	bool error = copyDirRecursively(work_dir, dest);
-	if (error) {
-		if (!tempname.isEmpty()) {
-			QDir().rename(tempname, dest);
-		}
-		return GenericRRequestResult::makeError(i18n("Error while copying %1 to %2", work_dir, dest));
+	KZip zip(tempname);
+	bool ok = zip.open(QIODevice::WriteOnly);
+	zip.addLocalDirectory(work_dir, "rkward_output");
+	ok = ok && zip.close();
+	if (!ok) {
+		QFile(tempname).remove();
+		return GenericRRequestResult::makeError(i18n("Error while writing temporary output archive %1", tempname));
 	}
-	if (!tempname.isEmpty()) {
-		QDir(tempname).removeRecursively();
+
+	if (QFile(dest).exists()) {
+		if (!QFile(dest).remove()) return GenericRRequestResult::makeError(i18n("Failure to remove existing file %1. Missing permissions?s", dest));
+	}
+	ok = QFile(tempname).copy(dest);
+	QFile(tempname).remove();
+	if (!ok) {
+		return GenericRRequestResult::makeError(i18n("Failure while copying output archive to %1", dest));
 	}
 
-	return GenericRRequestResult(QVariant(dest));  // return effective destination path. Needed by save()
+	return GenericRRequestResult(QFileInfo(dest).canonicalFilePath()); // return effective destination path. Needed by save()
 }
 
-GenericRRequestResult RKOutputDirectory::importInternal(const QString &_dir) {
+#include <KArchiveDirectory>
+GenericRRequestResult RKOutputDirectory::importZipInternal(const QString &_from) {
 	RK_TRACE (APP);
 
-	QFileInfo fi(_dir);
-	if (!fi.isDir()) {
-		return GenericRRequestResult::makeError(i18n("The path %1 does not exist or is not a directory.", _dir));
+	QFileInfo fi(_from);
+	if (!fi.isFile()) {
+		return GenericRRequestResult::makeError(i18n("The path %1 does not exist or is not a file.", _from));
 	}
-	QString dir = fi.canonicalFilePath ();
-
-	if (!copyDirRecursively(dir, work_dir)) {
-		QDir(work_dir).removeRecursively();
-		return GenericRRequestResult::makeError(i18n("The path %1 could not be imported (copy failure).", _dir));
+	QString from = fi.canonicalFilePath();
+
+	KZip zip(from);
+	bool ok = zip.open(QIODevice::ReadOnly);
+	if (ok) {
+		auto dir = zip.directory()->entry("rkward_output");
+		if (!(dir && dir->isDirectory())) ok = false;
+		if (ok && !static_cast<const KArchiveDirectory*>(dir)->copyTo(work_dir, true)) ok = false;
 	}
+	if (!ok) return GenericRRequestResult::makeError(i18n("Failure to open %1. Not an rkward output file?", from));
 
-	save_dir = dir;
+	save_filename = from;
 	updateSavedHash();
 	initialized = true;
 
@@ -216,18 +205,18 @@ GenericRRequestResult RKOutputDirectory::import(const QString& _dir) {
 		return GenericRRequestResult::makeError(i18n("Output directory %1 is already in use.", id));
 	}
 
-	return importInternal(_dir);
+	return importZipInternal(_dir);
 }
 
 GenericRRequestResult RKOutputDirectory::revert(OverwriteBehavior discard) {
 	RK_TRACE (APP);
 
-	if (save_dir.isEmpty()) {
+	if (save_filename.isEmpty()) {
 		return GenericRRequestResult::makeError(i18n("Output directory %1 has not previously been saved. Cannot revert.", id));
 	}
 	if (!isModified()) return GenericRRequestResult(id, i18n("Output directory %1 had no modifications. Nothing reverted.", id));
 	if (discard == Ask) {
-		if (KMessageBox::warningContinueCancel(RKWardMainWindow::getMain(), i18n("Reverting will destroy any changes, since the last time you saved (%1). Are you sure you want to proceed?"), save_timestamp.toString()) == KMessageBox::Continue) {
+		if (KMessageBox::warningContinueCancel(RKWardMainWindow::getMain(), i18n("Reverting will destroy any changes, since the last time you saved (%1). Are you sure you want to proceed?", save_timestamp.toString())) == KMessageBox::Continue) {
 			discard = Force;
 		}
 	}
@@ -235,7 +224,7 @@ GenericRRequestResult RKOutputDirectory::revert(OverwriteBehavior discard) {
 		return GenericRRequestResult::makeError(i18n("User cancelled."));
 	}
 
-	return importInternal(save_dir);
+	return importZipInternal(save_filename);
 }
 
 RKOutputDirectory* RKOutputDirectory::createOutputDirectoryInternal() {
@@ -250,11 +239,6 @@ RKOutputDirectory* RKOutputDirectory::createOutputDirectoryInternal() {
 	}
 	ddir.mkpath(destname);
 
-	QFile marker(destname + QStringLiteral("/rkward_output_marker.txt"));
-	marker.open(QIODevice::WriteOnly);
-	marker.write(i18n("This file is used to indicate that this directory is an ouptut directory created by RKWard (http://rkward.kde.org). Do not place any other contents in this directory, as the entire directory will be purged if/when overwriting the output.\nRKWard will ask you before purging this directory (unless explicitly told otherwise), but if you remove this file, RKWard will not purge this directory.\n").toLocal8Bit());
-	marker.close();
-
 	auto d = new RKOutputDirectory();
 	d->work_dir = QFileInfo(ddir.absoluteFilePath(destname)).canonicalFilePath();
 	d->id = d->work_dir;
@@ -305,7 +289,7 @@ GenericRRequestResult RKOutputDirectory::clear(OverwriteBehavior discard) {
 bool RKOutputDirectory::isEmpty() const {
 	RK_TRACE(APP);
 
-	if (!save_dir.isEmpty()) return false;  // we _could_ have saved an empty output, of course, but no worries about corner cases. In any doubt we return false.
+	if (!save_filename.isEmpty()) return false;  // we _could_ have saved an empty output, of course, but no worries about corner cases. In any doubt we return false.
 
 	if (!initialized) return true;
 	if (!isModified()) return true;   // because we have not saved/loaded this file, before, see above
@@ -320,7 +304,7 @@ bool RKOutputDirectory::isModified() const {
 
 QString RKOutputDirectory::caption() const {
 	RK_TRACE(APP);
-	if (!save_dir.isEmpty()) return QFileInfo(save_dir).fileName();
+	if (!save_filename.isEmpty()) return QFileInfo(save_filename).fileName();
 	return i18n("Unsaved output");
 }
 
@@ -381,12 +365,6 @@ QList<RKOutputDirectory*> RKOutputDirectory::modifiedOutputDirectories() {
 	return ret;
 }
 
-bool RKOutputDirectory::isRKWwardOutputDirectory(const QString& dir) {
-	RK_TRACE (APP);
-
-	return (QDir(dir).exists(QStringLiteral("rkward_output_marker.txt")));
-}
-
 void RKOutputDirectory::updateSavedHash() {
 	RK_TRACE (APP);
 	saved_hash = hashDirectoryState(work_dir);
@@ -497,9 +475,9 @@ GenericRRequestResult RKOutputDirectory::handleRCall(const QStringList& params,
 		} else if (command == QStringLiteral("revert")) {
 			return out->revert(parseOverwrite(params.value(2)));
 		} else if (command == QStringLiteral("save")) {
-			return out->save(params.value(2), parseOverwrite(params.value(3)));
+			return out->save(params.value(3), parseOverwrite(params.value(2)));
 		} else if (command == QStringLiteral("export")) {
-			return out->exportAs(params.value(2), parseOverwrite(params.value(3)));
+			return out->exportAs(params.value(3), parseOverwrite(params.value(2)));
 		} else if (command == QStringLiteral("clear")) {
 			return out->clear(parseOverwrite(params.value(2)));
 		} else if (command == QStringLiteral("close")) {
diff --git a/rkward/misc/rkoutputdirectory.h b/rkward/misc/rkoutputdirectory.h
index 41a18d33..0836bb91 100644
--- a/rkward/misc/rkoutputdirectory.h
+++ b/rkward/misc/rkoutputdirectory.h
@@ -49,7 +49,7 @@ public:
 	bool isActive() const;
 	bool isModified() const;
 	GenericRRequestResult view(bool raise);
-	QString filename() const { return save_dir; };
+	QString filename() const { return save_filename; };
 	QString workDir() const { return work_dir; }
 	QString workPath() const;
 	QString caption() const;
@@ -74,7 +74,7 @@ private:
 	QString saved_hash;
 	QDateTime save_timestamp;
 	QString work_dir;
-	QString save_dir;
+	QString save_filename;
 	QString id;
 	bool initialized;
 
@@ -84,8 +84,8 @@ private:
 
 	GenericRRequestResult import(const QString& from);
 	static RKOutputDirectory* createOutputDirectoryInternal();
-	static bool isRKWwardOutputDirectory (const QString &dir);
-	GenericRRequestResult importInternal(const QString &dir);
+	GenericRRequestResult importZipInternal(const QString &from);
+	GenericRRequestResult exportZipInternal(const QString &dest);
 };
 
 #endif
diff --git a/rkward/rbackend/rpackages/rkward/R/rk.output.R b/rkward/rbackend/rpackages/rkward/R/rk.output.R
index e3d72da5..b6c40d13 100644
--- a/rkward/rbackend/rpackages/rkward/R/rk.output.R
+++ b/rkward/rbackend/rpackages/rkward/R/rk.output.R
@@ -2,8 +2,8 @@
 #' @name RK.Output
 #'
 #' @description Since version 0.7.5, RKWard supports more than one piece of output. While dealing with only a single output page, there will be no need for the user to call any of
-#'              these functions, directly, as exactly one output
-#'              is always opened for writing in RKWard. However, these functions allow to manage several distinct output pages, programmatically.
+#'              these functions, directly, as exactly one output is always opened for writing in RKWard (unless rk.set.output.html.file() has been called, explicitly).
+#'              However, these functions allow to manage several distinct output pages, programmatically.
 #'
 #'              The primary entry point is the function \code{rk.output}, which allows to retrieve the current output directly, or to load or create a new one. This will return
 #'              an instance of the \code{RK.Output} reference class, which has methods for subsequent manipulation. Two instances of this class may be pointing to the same
@@ -59,12 +59,13 @@ RK.Output <- setRefClass(Class="RK.Output", fields=list(id="character"),
 		},
 		save=function(filename, overwrite=NULL) {
 "Save this output, either to the last known save location (if no filename is specified) or to a new location (\"save as\")."
-			.rk.do.call("output", c ("save", .checkId(), filename, if(is.null(overwrite)) "ask" else if(isTRUE(overwrite)) "force" else "fail"))
+			if (missing(filename)) filename <- ""
+			.rk.do.call("output", c ("save", .checkId(), if(is.null(overwrite)) "ask" else if(isTRUE(overwrite)) "force" else "fail", filename))
 		},
 		export=function(filename, overwrite=NULL) {
 "Save this output, to the specified location, but keep it associated with the previous location (\"save a copy\")."
 			if (missing(filename)) stop("No file name specified")
-			.rk.do.call("output", c ("export", .checkId(), filename, if(is.null(overwrite)) "ask" else if(isTRUE(overwrite)) "force" else "fail"))
+			.rk.do.call("output", c ("export", .checkId(), if(is.null(overwrite)) "ask" else if(isTRUE(overwrite)) "force" else "fail", filename))
 		},
 		clear=function(discard=NULL) {
 "Clear all content from this output. As with any function in this class, this affects the working copy, only, until you call save. Therefore, by default, the user will be prompted for confirmation




More information about the rkward-tracker mailing list