Commit 1cb9a213 authored by Nate Graham's avatar Nate Graham
Browse files

Display more informative and actionable error messages

These error messages were a bit short and contained technical jargon.
They did not give the user a good sense of how to proceed.

With this commit, the error messages are changed to clearly include
the important elements of a good error message:
1. What went wrong
2. Why it happened
3. How they can fix it themselves
4. How they can report the issue to get it permanently fixed for
   everyone

To do this, the setError() function is extended to also be able to
accept an optional explanation arguments, which is used to present
better information to the user.
parent e82bf26f
Pipeline #147442 passed with stage
in 2 minutes and 36 seconds
......@@ -2,7 +2,7 @@
# SPDX-FileCopyrightText: 2021 Harald Sitter <sitter@kde.org>
add_library(KInfoCenterInternal SHARED CommandOutputContext.cpp CommandOutputContext.h)
target_link_libraries(KInfoCenterInternal Qt::Qml KF5::I18n)
target_link_libraries(KInfoCenterInternal Qt::Qml KF5::I18n KF5::CoreAddons)
# Disable legacy stuff to get rid of some deprecation warnings. Notably duplicated QProcess::finished overloads.
target_compile_definitions(KInfoCenterInternal PRIVATE -DQT_DISABLE_DEPRECATED_BEFORE=0x050e00)
target_include_directories(KInfoCenterInternal INTERFACE "$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>")
......
......@@ -12,12 +12,14 @@
#include <QStandardPaths>
#include <KLocalizedString>
#include <KOSRelease>
CommandOutputContext::CommandOutputContext(const QStringList &findExecutables, const QString &executable, const QStringList &arguments, QObject *parent)
: QObject(parent)
, m_executableName(executable)
, m_executablePath(QStandardPaths::findExecutable(m_executableName))
, m_arguments(arguments)
, m_bugReportUrl(KOSRelease().bugReportUrl())
{
// Various utilities are installed in sbin, but work without elevated privileges
if (m_executablePath.isEmpty()) {
......@@ -74,10 +76,12 @@ void CommandOutputContext::reset()
{
m_ready = false;
m_error.clear();
m_explanation.clear();
m_text.clear();
m_filter.clear();
Q_EMIT readyChanged();
Q_EMIT errorChanged();
Q_EMIT explanationChanged();
Q_EMIT textChanged();
Q_EMIT filterChanged();
......@@ -91,7 +95,10 @@ void CommandOutputContext::load()
for (auto it = m_foundExecutablePaths.cbegin(); it != m_foundExecutablePaths.cend(); ++it) {
if (it.value().isEmpty()) {
setError(xi18nc("@info", "The executable <command>%1</command> could not be found in $PATH.", it.key()));
setError(xi18nc("@info", "The <command>%1</command> tool is required to display this page, but could not be found", it.key()),
xi18nc("@info",
"You can search for it and install it your using your package manager.<nl/>"
"Then please report this packaging issue to your distribution."));
return;
}
}
......@@ -103,7 +110,8 @@ void CommandOutputContext::load()
switch (exitStatus) {
case QProcess::CrashExit:
return setError(xi18nc("@info", "The subprocess <command>%1</command> crashed unexpectedly. No data could be obtained.", m_executableName));
return setError(xi18nc("@info", "The <command>%1</command> tool crashed while generating page content", m_executableName),
xi18nc("@Info", "Try again later. If keeps happening, please report the crash to your distribution."));
case QProcess::NormalExit:
break;
}
......@@ -122,10 +130,17 @@ void CommandOutputContext::load()
proc->start(m_executablePath, m_arguments);
}
void CommandOutputContext::setError(const QString &message)
void CommandOutputContext::setError(const QString &message, const QString &explanation = QString())
{
m_error = message;
if (!explanation.isEmpty()) {
m_explanation = explanation;
}
Q_EMIT errorChanged();
Q_EMIT explanationChanged();
setReady();
}
......
......@@ -7,6 +7,7 @@
#include <QMap>
#include <QObject>
#include <QUrl>
// Somewhat general-purpose command executor. This class runs the executable with arguments, collecting all its output
// and potentially filtering it to limit the lines to only ones matching.
......@@ -23,6 +24,10 @@ class CommandOutputContext : public QObject
Q_PROPERTY(bool ready MEMBER m_ready NOTIFY readyChanged)
// Potential error description. Empty when there is no error to report.
Q_PROPERTY(QString error MEMBER m_error NOTIFY errorChanged)
// Extra explanatory text for error conditions. Empty when no explanatory text has been specified.
Q_PROPERTY(QString explanation MEMBER m_explanation NOTIFY explanationChanged)
// URL where the user can report a bug when there is an error. Empty when there is no error, or no applicable place to report a bug
Q_PROPERTY(QUrl bugReportUrl MEMBER m_bugReportUrl CONSTANT)
public:
CommandOutputContext(const QStringList &findExecutables, const QString &executable, const QStringList &arguments, QObject *parent = nullptr);
CommandOutputContext(const QString &executable, const QStringList &arguments, QObject *parent = nullptr);
......@@ -38,22 +43,25 @@ Q_SIGNALS:
void textChanged();
void readyChanged();
void errorChanged();
void explanationChanged();
private:
void reset();
void load();
void setError(const QString &message);
void setError(const QString &message, const QString &explanation);
void setReady();
const QString m_executableName;
QString m_executablePath;
QMap<QString, QString> m_foundExecutablePaths;
const QStringList m_arguments;
const QUrl m_bugReportUrl;
QStringList m_originalLines;
bool m_ready = false;
QString m_error;
QString m_explanation;
QString m_text; // possibly filtered
QString m_filter;
......
......@@ -85,21 +85,38 @@ KCM.SimpleKCM {
id: noDataComponent
Kirigami.PlaceholderMessage {
width: root.width - (Kirigami.Units.largeSpacing * 4)
readonly property bool errorNotFilter: output.filter === "" && output.error !== ""
width: root.width - (Kirigami.Units.largeSpacing * 8)
// Can't do anchors.centerIn: parent here
y: root.height/2 - height/2
x: root.width/2 - width/2
text: {
if (output.filter !== "") {
return i18nc("@info", "No text matching the filter")
}
if (output.error !== "") {
return output.error
}
if (output.filter !== "" && output.text === "") {
return i18nc("@info", "No text matching the filter")
}
return i18nc("@info the KCM has no data to display", "No data available")
}
explanation: {
if (errorNotFilter && output.explanation !== "") {
return output.explanation
}
return ""
}
icon.name: "data-warning"
helpfulAction: Kirigami.Action {
enabled: errorNotFilter
icon.name: "tools-report-bug"
text: i18n("Report this issue")
onTriggered: {
Qt.openUrlExternally(output.bugReportUrl)
}
}
}
}
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment