[education/rkward] rkward: Remove unused code in RKProgressControl, be less alarmist about stderr output
Thomas Friedrichsmeier
null at kde.org
Thu Aug 8 20:55:21 BST 2024
Git commit d9ce135c507f8dba1959252760a85e52bd5c62fe by Thomas Friedrichsmeier.
Committed on 08/08/2024 at 19:55.
Pushed by tfry into branch 'master'.
Remove unused code in RKProgressControl, be less alarmist about stderr output
M +33 -66 rkward/misc/rkprogresscontrol.cpp
M +13 -20 rkward/misc/rkprogresscontrol.h
M +2 -0 rkward/rbackend/rcommand.h
M +3 -0 rkward/rbackend/rpackages/rkward/R/rk.with.progress.R
M +1 -1 rkward/windows/rkwindowcatcher.cpp
https://invent.kde.org/education/rkward/-/commit/d9ce135c507f8dba1959252760a85e52bd5c62fe
diff --git a/rkward/misc/rkprogresscontrol.cpp b/rkward/misc/rkprogresscontrol.cpp
index 2cd30d03e..274a1fe6e 100644
--- a/rkward/misc/rkprogresscontrol.cpp
+++ b/rkward/misc/rkprogresscontrol.cpp
@@ -35,6 +35,7 @@ public:
public:
void addOutput (const ROutput *output);
void finished ();
+ void indicateError();
protected:
void closeEvent (QCloseEvent *e) override;
void scrollDown ();
@@ -108,54 +109,22 @@ void RKProgressControl::doNonModal (bool autodelete) {
}
}
-void RKProgressControl::newError (const QString &error) {
- RK_TRACE (MISC);
-
- if (!(mode & IncludeErrorOutput)) return;
-
- ROutput outputc;
- outputc.type = ROutput::Error;
- outputc.output = error;
-
- output_log.append (outputc);
-
- if (mode & RaiseOnError) {
- if (!dialog) createDialog ();
- dialog->raise ();
- }
- if (dialog) dialog->addOutput (&outputc);
-}
-
-void RKProgressControl::newOutput (const QString &output) {
- RK_TRACE (MISC);
-
- if (!(mode & IncludeRegularOutput)) return;
-
- ROutput outputc;
- outputc.type = ROutput::Output;
- outputc.output = output;
-
- output_log.append (outputc);
-
- if (mode & RaiseOnRegularOutput) {
- if (!dialog) createDialog ();
- dialog->raise ();
- }
- if (dialog) dialog->addOutput (&outputc);
-}
-
void RKProgressControl::addRCommand (RCommand *command, bool done_when_finished) {
RK_TRACE (MISC);
RK_ASSERT (command);
outstanding_commands.append(command);
- connect(command->notifier(), &RCommandNotifier::commandOutput, this, QOverload<RCommand*, const ROutput*>::of(&RKProgressControl::newOutput));
- if (done_when_finished) {
- command->whenFinished(this, [this](RCommand* command) {
- outstanding_commands.removeAll(command);
- done();
- });
- }
+ connect(command->notifier(), &RCommandNotifier::commandOutput, this, &RKProgressControl::newOutput);
+ command->whenFinished(this, [this, done_when_finished](RCommand* command) {
+qDebug("finished");
+ if (command->failed()) {
+qDebug("failed");
+ if (!dialog) createDialog();
+ dialog->indicateError();
+ }
+ outstanding_commands.removeAll(command);
+ if (done_when_finished) done();
+ });
}
void RKProgressControl::dialogDestroyed () {
@@ -202,11 +171,13 @@ void RKProgressControl::newOutput (RCommand *, const ROutput *output) {
RK_TRACE (MISC);
RK_ASSERT (output);
- if (output->type == ROutput::Output) {
- newOutput (output->output);
- } else {
- newError (output->output);
+ const ROutput outputc(*output);
+ output_log.append(outputc);
+ if (mode & RaiseOnOutput) {
+ if (!dialog) createDialog();
+ dialog->raise();
}
+ if (dialog) dialog->addOutput(output);
}
QString RKProgressControl::fullCommandOutput() {
@@ -242,27 +213,18 @@ RKProgressControlDialog::RKProgressControlDialog(const QString &text, const QStr
mainboxlayout->addWidget (RKCommonFunctions::linkedWrappedLabel (text));
- error_indicator = new QLabel (i18n ("<b>There have been errors and / or warnings. See below for a transcript</b>"), mainbox);
- QPalette palette = error_indicator->palette ();
- palette.setColor (error_indicator->foregroundRole (), QColor (255, 0, 0));
- error_indicator->setPalette (palette);
- error_indicator->hide ();
- mainboxlayout->addWidget (error_indicator);
+ error_indicator = new QLabel(i18n("<b>There has been an error while running the command. See below for output details</b>"), mainbox);
+ QPalette palette = error_indicator->palette();
+ palette.setColor(error_indicator->foregroundRole(), QColor(255, 0, 0));
+ error_indicator->setPalette(palette);
+ error_indicator->hide();
+ mainboxlayout->addWidget(error_indicator);
detailsbox = new QWidget (this);
QVBoxLayout *detailsboxlayout = new QVBoxLayout (detailsbox);
detailsboxlayout->setContentsMargins (0, 0, 0, 0);
- QString ocaption;
- if (mode_flags & RKProgressControl::IncludeRegularOutput) {
- output_button_text = i18n ("Output");
- ocaption = i18n ("Output:");
- } else {
- output_button_text = i18n ("Errors / Warnings");
- ocaption = i18n ("Errors / Warnings:");
- }
- QLabel* label = new QLabel (ocaption, detailsbox);
- detailsboxlayout->addWidget (label);
+ detailsboxlayout->addWidget(new QLabel(i18n("Output")));
output_text = new QTextEdit (detailsbox);
output_text->setReadOnly (true);
@@ -275,8 +237,8 @@ RKProgressControlDialog::RKProgressControlDialog(const QString &text, const QStr
buttons = new QDialogButtonBox (QDialogButtonBox::Cancel, this);
connect (buttons->button (QDialogButtonBox::Cancel), &QPushButton::clicked, this, &QDialog::reject);
- if (mode_flags & RKProgressControl::OutputSwitchable) {
- QPushButton* button = buttons->addButton (output_button_text, QDialogButtonBox::HelpRole);
+ if ((mode_flags & RKProgressControl::ShowOutput) && !(mode_flags & RKProgressControl::OutputShownByDefault)) {
+ QPushButton* button = buttons->addButton(i18n("Show Output"), QDialogButtonBox::HelpRole);
connect (button, &QPushButton::clicked, this, &RKProgressControlDialog::toggleDetails);
}
if (mode_flags & RKProgressControl::AllowCancel) buttons->button (QDialogButtonBox::Cancel)->setText (i18n ("Cancel"));
@@ -298,6 +260,12 @@ RKProgressControlDialog::~RKProgressControlDialog () {
RK_TRACE (MISC);
}
+void RKProgressControlDialog::indicateError() {
+ RK_TRACE(MISC);
+ raise();
+ error_indicator->show();
+}
+
void RKProgressControlDialog::addOutput (const ROutput *output) {
RK_TRACE (MISC);
@@ -317,7 +285,6 @@ void RKProgressControlDialog::addOutput (const ROutput *output) {
} else {
output_text->setTextColor(RKStyle::viewScheme()->foreground(KColorScheme::NegativeText).color());
if (!detailsbox->isVisible ()) toggleDetails ();
- error_indicator->show ();
}
}
diff --git a/rkward/misc/rkprogresscontrol.h b/rkward/misc/rkprogresscontrol.h
index c1b184577..04af578ae 100644
--- a/rkward/misc/rkprogresscontrol.h
+++ b/rkward/misc/rkprogresscontrol.h
@@ -1,6 +1,6 @@
/*
rkprogresscontol - This file is part of the RKWard project. Created: Sun Sep 10 2006
-SPDX-FileCopyrightText: 2006-2012 by Thomas Friedrichsmeier <thomas.friedrichsmeier at kdemail.net>
+SPDX-FileCopyrightText: 2006-2024 by Thomas Friedrichsmeier <thomas.friedrichsmeier at kdemail.net>
SPDX-FileContributor: The RKWard Team <rkward-devel at kde.org>
SPDX-License-Identifier: GPL-2.0-or-later
*/
@@ -41,22 +41,18 @@ public:
/** These flags control the mode of operation. Generally you will use on of the predefined sets (StandardCancel, StandardError, DetailedError, StandardProgress, or CancellableProgress) */
enum RKProgressControlFlags {
- AllowCancel=1, /**< Show a cancel button. When the cancel button is pressed or the dialog is closed, the signal cancelled () is emitted, and (if invoked that way) doModal returns QDialog::rejected */
- AutoCancelCommands=1024, /**< if the user cancels the dialog, automatically cancel all commands previously added via addRCommand () (and not yet finished. Only meaningful if AllowCancel is set as well */
- IncludeErrorOutput=2, /**< Include erros output in the output shown */
- IncludeRegularOutput=4, /**< Include regular (no error) output in the output shown */
- RaiseOnError=16, /**< dialog is shown/raised, when there are errors. Only meaningful, if IncludeErrorOutput is set as well */
- RaiseOnRegularOutput=32, /**< dialog is also shown/raised, when there is new regular output. Only meaningful, if IncludeRegularOutput is set as well */
- OutputShownByDefault=64, /**< the textfield with the output is shown by default, not only when requested by the user. Requires at least one of IncludeErrorOutput or IncludeRegularOutput */
- OutputSwitchable=128, /**< the textfield with the output can be shown/hidden by the user */
- ShowAtOnce=256, /**< dialog is shown at once, instead of only when there is an error/output */
- PreventClose=512, /**< do not accept close events */
- StandardError=IncludeErrorOutput | RaiseOnError | OutputShownByDefault,
- DetailedError=StandardError | IncludeRegularOutput,
- StandardProgress=DetailedError | OutputSwitchable | RaiseOnRegularOutput,
- CancellableProgress=(StandardProgress | AllowCancel | AutoCancelCommands | PreventClose | ShowAtOnce) - (RaiseOnRegularOutput | OutputShownByDefault),
- CancellableNoProgress=CancellableProgress - OutputSwitchable,
- SimpleSplash=(StandardProgress | PreventClose | ShowAtOnce) - (RaiseOnRegularOutput | OutputShownByDefault | OutputSwitchable)
+ AllowCancel=1, /**< Show a cancel button. When the cancel button is pressed or the dialog is closed, the signal cancelled () is emitted, and (if invoked that way) doModal returns QDialog::rejected */
+ AutoCancelCommands=1024, /**< if the user cancels the dialog, automatically cancel all commands previously added via addRCommand () (and not yet finished. Only meaningful if AllowCancel is set as well */
+ ShowOutput=2, /**< Shown comand output */
+ RaiseOnOutput=4, /**< dialog is shown/raised, when there is new output (needs ShowOutput) */
+ OutputShownByDefault=32, /**< the textfield with the output is shown by default, not only when requested by the user. Requires ShowOutput */
+ ShowAtOnce=256, /**< dialog is shown at once, instead of only when there is an error/output */
+ PreventClose=512, /**< do not accept close events */
+ StandardProgress=ShowOutput | OutputShownByDefault | RaiseOnOutput | ShowAtOnce,
+ ErrorsOnly=StandardProgress - ShowAtOnce,
+ CancellableProgress=(StandardProgress | AllowCancel | AutoCancelCommands | PreventClose | ShowAtOnce) - (OutputShownByDefault),
+ CancellableNoProgress=CancellableProgress - ShowOutput,
+ SimpleSplash=(StandardProgress | PreventClose | ShowAtOnce) - (RaiseOnOutput | OutputShownByDefault)
};
/** show the dialog modal. This will always show the dialog right away
@@ -78,9 +74,6 @@ public Q_SLOTS:
void dialogDestroyed ();
/** the corresponding action has finished. If there have been no errors, the dialog is also closed. Otherwise, the text of the "cancel" button is changed to "finished". */
void done ();
- void newError (const QString &error);
-/** usually you will call newError instead. However, if in case of an error, you also want to show the regular output, use this function to add output. The output is added to the internal error_log, but the dialog is not shown until you call newError (). */
- void newOutput (const QString &output);
private:
void createDialog ();
diff --git a/rkward/rbackend/rcommand.h b/rkward/rbackend/rcommand.h
index 3aec00593..8f5580f0b 100644
--- a/rkward/rbackend/rcommand.h
+++ b/rkward/rbackend/rcommand.h
@@ -44,6 +44,8 @@ friend class RCommandStackModel;
/** this struct is used to store the R output to an RCommand. The RCommand basically keeps a list of ROutputString (s). The difference to a normal
QString is, that additionally we store information on whether the output was "normal", "warning", or an "error". */
struct ROutput {
+ ROutput(const ROutput &other) : type(other.type), output(other.output) {};
+ ROutput() : type(NoOutput) {};
enum ROutputType {
NoOutput, /**< No output. Rarely used. */
Output, /**< normal output */
diff --git a/rkward/rbackend/rpackages/rkward/R/rk.with.progress.R b/rkward/rbackend/rpackages/rkward/R/rk.with.progress.R
index 17757c746..afbb9f04d 100644
--- a/rkward/rbackend/rpackages/rkward/R/rk.with.progress.R
+++ b/rkward/rbackend/rpackages/rkward/R/rk.with.progress.R
@@ -52,5 +52,8 @@ rk.with.progress <- function(expr, text="") {
expr=eval(.rk.variables$with.progress.expr, .rk.variables$with.progress.env),
error=function(e) { .rk.variables$with.progress.err <- e }
)
+ # NOTE we need to signal any error both here (to inform the frontend), and in rk.with.progress() (to relay the failure)
+ # it is a known (but thought-to-be-acceptable) problem that this results in duplicate printing of the error message
+ if (!is.null(.rk.variables$with.progress.err)) stop(.rk.variables$with.progress.err)
invisible(NULL)
}
diff --git a/rkward/windows/rkwindowcatcher.cpp b/rkward/windows/rkwindowcatcher.cpp
index 29ab8407e..02da6610d 100644
--- a/rkward/windows/rkwindowcatcher.cpp
+++ b/rkward/windows/rkwindowcatcher.cpp
@@ -307,7 +307,7 @@ void RKCaughtX11Window::commonInit (int device_number) {
RK_ASSERT (!device_windows.contains (device_number));
device_windows.insert (device_number, this);
- error_dialog = new RKProgressControl(nullptr, i18n("An error occurred"), i18n("An error occurred"), RKProgressControl::DetailedError);
+ error_dialog = new RKProgressControl(nullptr, i18n("An error occurred"), i18n("An error occurred"), RKProgressControl::ErrorsOnly);
setPart (new RKCaughtX11WindowPart (this));
setMetaInfo(i18n("Graphics Device Window"), QUrl("rkward://page/rkward_plot_history"), RKSettingsModuleGraphics::page_id);
initializeActivationSignals ();
More information about the rkward-tracker
mailing list