[education/rkward] rkward: More robust edit-prevention of broken data.frames.
Thomas Friedrichsmeier
null at kde.org
Sun Jul 14 10:54:52 BST 2024
Git commit 9e8e497793bc28f665eaa85325ca84741d866179 by Thomas Friedrichsmeier.
Committed on 14/07/2024 at 09:54.
Pushed by tfry into branch 'master'.
More robust edit-prevention of broken data.frames.
M +8 -0 rkward/autotests/core_test.cpp
M +18 -32 rkward/dataeditor/rkvareditmodel.cpp
M +2 -6 rkward/dataeditor/rkvareditmodel.h
M +45 -25 rkward/dataeditor/twintable.cpp
M +4 -2 rkward/dataeditor/twintable.h
https://invent.kde.org/education/rkward/-/commit/9e8e497793bc28f665eaa85325ca84741d866179
diff --git a/rkward/autotests/core_test.cpp b/rkward/autotests/core_test.cpp
index 6c8aa1f8d..225868c97 100644
--- a/rkward/autotests/core_test.cpp
+++ b/rkward/autotests/core_test.cpp
@@ -463,6 +463,14 @@ private Q_SLOTS:
waitForAllFinished();
}
+ void dataEditorTest() {
+ // Create a quirky data.frame, intentionally. Goal is not to crash ;-)
+ RInterface::issueCommand(new RCommand("df <- data.frame('a'=letters, 'a'=letters, 'b'=letters, check.names=FALSE); df[[2, 'b']] <- list('x')", RCommand::User));
+ RInterface::issueCommand(new RCommand("rk.edit(df)", RCommand::User));
+ waitForAllFinished();
+ RKWardMainWindow::getMain()->slotCloseAllEditors();
+ }
+
void restartRBackend() {
RInterface::issueCommand(new RCommand("setwd(tempdir())", RCommand::User)); // retart used to fail, if in non-existant directory
RInterface::issueCommand(new RCommand("x <- 1", RCommand::User));
diff --git a/rkward/dataeditor/rkvareditmodel.cpp b/rkward/dataeditor/rkvareditmodel.cpp
index 577fe4e98..b2a56b475 100644
--- a/rkward/dataeditor/rkvareditmodel.cpp
+++ b/rkward/dataeditor/rkvareditmodel.cpp
@@ -28,7 +28,6 @@ RKVarEditModel::RKVarEditModel (QObject *parent) : RKVarEditModelBase (parent),
edit_blocks = 0;
rownames = nullptr;
header_locked = false;
- duplicate_check_triggered = false;
reset_scheduled = false;
addNotificationType (RObjectListener::ObjectRemoved);
@@ -66,7 +65,23 @@ void RKVarEditModel::addObject (int index, RKVariable* object) {
if (meta_model) meta_model->endAddDataObject ();
endInsertColumns ();
- checkDuplicates ();
+ auto name = object->getShortName();
+ if (index >= var_col_offset) {
+ for (int i = var_col_offset; i < objects.size(); ++i) {
+ if (i == index) continue;
+ if (objects[i]->getShortName() == name) {
+ addProblem(i18n("Duplicate column name '%1'", name));
+ }
+ }
+ }
+}
+
+void RKVarEditModel::addProblem(const QString &prob) {
+ RK_TRACE(EDITOR);
+ if (problem_details.isEmpty()) {
+ Q_EMIT hasProblems();
+ }
+ problem_details.append(prob);
}
void RKVarEditModel::objectRemoved (RObject* object) {
@@ -88,35 +103,6 @@ void RKVarEditModel::objectRemoved (RObject* object) {
if (objects.size () <= var_col_offset) Q_EMIT modelDepleted(); // editor may or may want to auto-destruct
}
-void RKVarEditModel::checkDuplicates () {
- RK_TRACE (EDITOR);
-
- if (duplicate_check_triggered) return;
- duplicate_check_triggered = true;
- QTimer::singleShot(0, this, &RKVarEditModel::checkDuplicatesNow);
-}
-
-void RKVarEditModel::checkDuplicatesNow () {
- RK_TRACE (EDITOR);
-
- duplicate_check_triggered = false;
-
- QStringList dupes;
- for (int i = var_col_offset; i < objects.size (); ++i) {
- QString name = objects[i]->getShortName ();
- for (int j = i+1; j < objects.size (); ++j) {
- if (objects[j]->getShortName () == name) {
- if (objects[i]->getFullName () == objects[j]->getFullName ()) {
- dupes.append (objects[i]->getFullName ());
- j = objects.size (); // break
- }
- }
- }
- }
-
- if (!dupes.isEmpty ()) Q_EMIT hasDuplicates(dupes);
-}
-
void RKVarEditModel::objectMetaChanged (RObject* changed) {
RK_TRACE (EDITOR);
@@ -836,7 +822,7 @@ void RKVarEditDataFrameModel::init (RContainerObject* dataframe) {
if (obj->isVariable()) {
addObject(i, static_cast<RKVariable*>(obj));
} else {
- problem_details.append(i18n("Object '%1' is not a vector", obj->getShortName()));
+ addProblem(i18n("Object '%1' is not a vector", obj->getShortName()));
}
}
rownames = dataframe->rowNames ();
diff --git a/rkward/dataeditor/rkvareditmodel.h b/rkward/dataeditor/rkvareditmodel.h
index 075ef7e62..7eb61dfb1 100644
--- a/rkward/dataeditor/rkvareditmodel.h
+++ b/rkward/dataeditor/rkvareditmodel.h
@@ -85,9 +85,8 @@ public:
QStringList problems() const { return problem_details; };
Q_SIGNALS:
void modelDepleted ();
- void hasDuplicates (const QStringList& dupes);
+ void hasProblems();
private Q_SLOTS:
- void checkDuplicatesNow ();
void doResetNow ();
private:
bool reset_scheduled;
@@ -97,6 +96,7 @@ friend class RKVarEditMetaModel;
QList<RKVariable*> objects;
QStringList problem_details;
RKRowNames *rownames;
+ void addProblem(const QString &desc);
/** very simple convenience function to return the number of true cols + trailing cols */
int apparentCols () const { return (trueCols () + trailing_cols); };
@@ -112,10 +112,6 @@ friend class RKVarEditMetaModel;
virtual void doInsertRowsInBackend (int row, int count);
virtual void doRemoveRowsInBackend (int row, int count);
- /** Check whether there are any duplicate names in the model. Actual check is delayed until the next iteration of the event loop. If a duplicate is found, hasDuplicates(const QStringList&) is emitted. */
- void checkDuplicates ();
- bool duplicate_check_triggered;
-
int trailing_rows;
int trailing_cols;
diff --git a/rkward/dataeditor/twintable.cpp b/rkward/dataeditor/twintable.cpp
index 997667d0c..7ee66cfc5 100644
--- a/rkward/dataeditor/twintable.cpp
+++ b/rkward/dataeditor/twintable.cpp
@@ -12,6 +12,7 @@ SPDX-License-Identifier: GPL-2.0-or-later
#include <kactioncollection.h>
#include <kxmlguifactory.h>
#include <kmessagebox.h>
+#include <KMessageWidget>
#include <ktoggleaction.h>
#include <qvariant.h>
@@ -31,13 +32,19 @@ SPDX-License-Identifier: GPL-2.0-or-later
#include "../debug.h"
-TwinTable::TwinTable (QWidget *parent) : RKEditor (parent), RObjectListener (RObjectListener::Other), KXMLGUIClient () {
+TwinTable::TwinTable (QWidget *parent) :
+ RKEditor(parent),
+ RObjectListener(RObjectListener::Other),
+ KXMLGUIClient(),
+ datamodel(nullptr),
+ problems_note(nullptr),
+ main_object(nullptr)
+{
RK_TRACE (EDITOR);
- main_object = nullptr;
-
+ // NOTE: Must have an outer QVBoxLayout! Assumed in hasProblems()
QVBoxLayout *layout = new QVBoxLayout(this);
- layout->setContentsMargins (0, 0, 0, 0);
+ layout->setContentsMargins(0, 0, 0, 0);
splitter = new QSplitter (this);
splitter->setOrientation (Qt::Vertical);
@@ -188,11 +195,8 @@ void TwinTable::initTable (RKVarEditModel* model, RObject* object) {
addNotificationType (RObjectListener::MetaChanged);
listenForObject (object);
objectMetaChanged (object);
- connect (model, &RKVarEditModel::hasDuplicates, this, &TwinTable::containsDuplicates);
-
- if (!problems.isEmpty()) {
- KMessageBox::detailedError(this, i18n("The object's internal structure does not conform to a regular <tt>data.frame</tt>. Editing has been disabled, and some columns may not be shown. It may be possible to convert this object to a regular <tt>data.frame</tt> using <tt>as.data.frame()</tt>."), problems.join("\n"), i18n("Problem detected"));
- }
+ connect (model, &RKVarEditModel::hasProblems, this, &TwinTable::hasProblems);
+ if (!problems.isEmpty()) hasProblems();
}
void TwinTable::setWindowStyleHint (const QString& hint) {
@@ -207,12 +211,24 @@ void TwinTable::setWindowStyleHint (const QString& hint) {
RKMDIWindow::setWindowStyleHint (hint);
}
-void TwinTable::containsDuplicates (const QStringList& dupes) {
+void TwinTable::hasProblems() {
RK_TRACE (EDITOR);
- if (!rw) return;
- KMessageBox::informationList (this, i18n ("The editor '%1' contains the following duplicate columns. Editing this table may not be safe, and has been disabled. You may re-enable editing if you know what you are doing, but you are strongly advised to fix the table, and/or backup your data, first.", windowTitle ()), dupes, i18n ("Duplicate columns detected"));
- enableEditing (false);
+ if (!problems_note) {
+ const QString &msg = i18n("The object's internal structure does not conform to a regular <tt>data.frame</tt>. Editing this table may not be safe, and has been disabled. Some columns may not be shown.");
+ problems_note = new KMessageWidget(msg);
+ problems_note->setMessageType(KMessageWidget::Error);
+ static_cast<QVBoxLayout*>(layout())->insertWidget(0, problems_note);
+ auto a = new QAction(problems_note);
+ a->setText(i18n("Show details"));
+ problems_note->addAction(a);
+ connect(a, &QAction::triggered, datamodel, [this, msg]() {
+ // NOTE: Not caching the problems in the lambda, as additional problem may yet be detected
+ KMessageBox::detailedError(this, msg + "<br/>" + i18n("It may be possible to convert this object to a regular <tt>data.frame</tt> using <tt>as.data.frame()</tt>."), datamodel->problems().join("\n"), i18n("Problem detected"));
+ });
+ }
+ problems_note->animatedShow();
+ enableEditing(false);
}
void TwinTable::objectMetaChanged (RObject* changed) {
@@ -480,27 +496,31 @@ void TwinTable::flushEdit () {
void TwinTable::enableEditing (bool on) {
RK_TRACE (EDITOR);
- flushEdit ();
-
- rw = main_object ? on && main_object->canWrite () : on; // NOTE: File->New->Dataset creates an Editor window, first, then sets the object to edit afterwards (TODO which looks like silly design)
+ flushEdit();
+ // NOTE: File->New->Dataset creates an Editor window, first, then sets the object to edit afterwards (TODO which looks like silly design)
+ bool prevent_edit = (main_object && !main_object->canWrite()) || (datamodel && !datamodel->problems().isEmpty());
+ rw = on && !prevent_edit;
metaview->rw = rw;
dataview->rw = rw;
- QPalette palette = metaview->palette ();
- if (on) palette.setColor (QPalette::Base, QApplication::palette ().color (QPalette::Active, QPalette::Base));
- else palette.setColor (QPalette::Base, QApplication::palette ().color (QPalette::Disabled, QPalette::Base));
- metaview->setPalette (palette);
- dataview->setPalette (palette);
+ QPalette palette = metaview->palette();
+ if (rw) palette.setColor(QPalette::Base, QApplication::palette().color(QPalette::Active, QPalette::Base));
+ else palette.setColor(QPalette::Base, QApplication::palette().color(QPalette::Disabled, QPalette::Base));
+ metaview->setPalette(palette);
+ dataview->setPalette(palette);
QAbstractItemView::EditTriggers triggers = QAbstractItemView::NoEditTriggers;
if (rw) triggers = QAbstractItemView::DoubleClicked | QAbstractItemView::EditKeyPressed | QAbstractItemView::AnyKeyPressed;
metaview->setEditTriggers (triggers);
dataview->setEditTriggers (triggers);
- edit_actions->setEnabled (rw);
- action_enable_editing->setChecked (rw);
- action_tb_lock_editing->setChecked (!rw);
- action_tb_unlock_editing->setChecked (rw);
+ edit_actions->setEnabled(rw);
+ action_enable_editing->setChecked(rw);
+ action_enable_editing->setEnabled(!prevent_edit);
+ action_tb_lock_editing->setChecked(!rw);
+ action_tb_lock_editing->setEnabled(!prevent_edit);
+ action_tb_unlock_editing->setChecked(rw);
+ action_tb_unlock_editing->setEnabled(!prevent_edit);
if (main_object) objectMetaChanged (main_object); // update_caption;
}
diff --git a/rkward/dataeditor/twintable.h b/rkward/dataeditor/twintable.h
index 95aa7535f..5fcae1629 100644
--- a/rkward/dataeditor/twintable.h
+++ b/rkward/dataeditor/twintable.h
@@ -22,6 +22,7 @@ class RKVarEditModel;
class QActionGroup;
class QAction;
class QSplitter;
+class KMessageWidget;
/**
*@author Thomas Friedrichsmeier
@@ -64,8 +65,8 @@ public Q_SLOTS:
void pasteToTable();
/** paste the clipboard into the table, but not beyond selection boundaries */
void pasteToSelection();
-/** connected to RKVarEditModel::hasDuplicates() */
- void containsDuplicates (const QStringList& dupes);
+/** connected to RKVarEditModel::hasProblems() */
+ void hasProblems();
private:
int meta_header_anchor_section;
/** read-write */
@@ -83,6 +84,7 @@ protected:
/** Returns the active Table (of the two members), 0 if no table active */
TwinTableMember *activeTable ();
+ KMessageWidget* problems_note;
TwinTableMember* metaview;
TwinTableMember* dataview;
More information about the rkward-tracker
mailing list