[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [kcm-grub2] src: TODO--: Fix the encoding issues in the helper the right way.
From:       Konstantinos Smanis <konstantinos.smanis () gmail ! com>
Date:       2013-02-06 21:56:17
Message-ID: 20130206215617.67F14A6091 () git ! kde ! org
[Download RAW message or body]

Git commit 8e180199da0f7298988a4ace4d3b23fa3c130cbf by Konstantinos Smanis.
Committed on 06/02/2013 at 22:49.
Pushed by ksmanis into branch 'master'.

TODO--: Fix the encoding issues in the helper the right way.

The helper doesn't need to know the system's encoding. Just pass raw
data back and forth and let the main application en-/de-code them.

Hopefully a better solution that the previous (UTF-8) hardcoded values.

Tested on Gentoo and Kubuntu 12.10 with UTF-8 encoding. I hope I haven't
missed anything too big.

M  +15   -77   src/helper/helper.cpp
M  +43   -8    src/kcm_grub2.cpp
M  +5    -0    src/kcm_grub2.h

http://commits.kde.org/kcm-grub2/8e180199da0f7298988a4ace4d3b23fa3c130cbf

diff --git a/src/helper/helper.cpp b/src/helper/helper.cpp
index 19c5903..5136cc7 100644
--- a/src/helper/helper.cpp
+++ b/src/helper/helper.cpp
@@ -23,13 +23,9 @@
 
 //Qt
 #include <QDir>
-#include <QTextCodec>
-#include <QTextStream>
 
 //KDE
 #include <KDebug>
-#include <kdeversion.h>
-#include <KGlobal>
 #include <KLocale>
 #include <KProcess>
 #include <KStandardDirs>
@@ -60,51 +56,24 @@ Helper::Helper()
     } else {
         qputenv("PATH", path.toLatin1());
     }
-    //TODO: system encoding should be sent from the core application
-    QTextCodec::setCodecForLocale(QTextCodec::codecForName("UTF-8"));
 }
 
 ActionReply Helper::executeCommand(const QStringList &command)
 {
-    ActionReply reply;
-    reply.addData("command", command);
-
     KProcess process;
     process.setProgram(command);
     process.setOutputChannelMode(KProcess::MergedChannels);
 
     kDebug() << "Executing" << command.join(" ");
     int exitCode = process.execute();
-    reply.addData("exitCode", exitCode);
-    QString output = QString::fromUtf8(process.readAll());
-    reply.addData("output", output);
 
-    if (exitCode == 0) {
-        return reply;
-    }
-
-    QString errorMessage;
-    switch (exitCode) {
-    case -2:
-        errorMessage = i18nc("@info", "The process could not be started.");
-        break;
-    case -1:
-        errorMessage = i18nc("@info", "The process crashed.");
-        break;
-    default:
-        errorMessage = output;
-        break;
+    ActionReply reply;
+    if (exitCode != 0) {
+        reply = ActionReply::HelperErrorReply;
+        reply.setErrorCode(exitCode);
     }
-    QString errorDescription = i18nc("@info", "Command: \
<command>%1</command><nl/>Error code: <numid>%2</numid><nl/>Error \
                message:<nl/><message>%3</message>", command.join(" "), exitCode, \
                errorMessage);
-
-    reply = ActionReply::HelperErrorReply;
-    reply.setErrorCode(exitCode);
-    reply.addData("errorMessage", errorMessage);
-#if KDE_IS_VERSION(4,7,0)
-    reply.setErrorDescription(errorDescription);
-#else
-    reply.addData("errorDescription", errorDescription);
-#endif
+    reply.addData("command", command);
+    reply.addData("output", process.readAll());
     return reply;
 }
 QString Helper::findShell()
@@ -136,32 +105,17 @@ ActionReply Helper::defaults(QVariantMap args)
 
     if (!QFile::exists(originalConfigFileName)) {
         reply = ActionReply::HelperErrorReply;
-        QString errorDescription = i18nc("@info", "Original configuration file \
                <filename>%1</filename> does not exist.", originalConfigFileName);
-#if KDE_IS_VERSION(4,7,0)
-        reply.setErrorDescription(errorDescription);
-#else
-        reply.addData("errorDescription", errorDescription);
-#endif
+        reply.addData("errorDescription", i18nc("@info", "Original configuration \
file <filename>%1</filename> does not exist.", originalConfigFileName));  return \
reply;  }
     if (!QFile::remove(configFileName)) {
         reply = ActionReply::HelperErrorReply;
-        QString errorDescription = i18nc("@info", "Cannot remove current \
                configuration file <filename>%1</filename>.", configFileName);
-#if KDE_IS_VERSION(4,7,0)
-        reply.setErrorDescription(errorDescription);
-#else
-        reply.addData("errorDescription", errorDescription);
-#endif
+        reply.addData("errorDescription", i18nc("@info", "Cannot remove current \
configuration file <filename>%1</filename>.", configFileName));  return reply;
     }
     if (!QFile::copy(originalConfigFileName, configFileName)) {
         reply = ActionReply::HelperErrorReply;
-        QString errorDescription = i18nc("@info", "Cannot copy original \
configuration file <filename>%1</filename> to <filename>%2</filename>.", \
                originalConfigFileName, configFileName);
-#if KDE_IS_VERSION(4,7,0)
-        reply.setErrorDescription(errorDescription);
-#else
-        reply.addData("errorDescription", errorDescription);
-#endif
+        reply.addData("errorDescription", i18nc("@info", "Cannot copy original \
configuration file <filename>%1</filename> to <filename>%2</filename>.", \
originalConfigFileName, configFileName));  return reply;
     }
     return reply;
@@ -178,12 +132,7 @@ ActionReply Helper::install(QVariantMap args)
         for (int i = 0; QDir(mountPoint = \
QString("%1/kcm-grub2-%2").arg(QDir::tempPath(), QString::number(i))).exists(); i++); \
if (!QDir().mkpath(mountPoint)) {  reply = ActionReply::HelperErrorReply;
-            QString errorDescription = i18nc("@info", "Failed to create temporary \
                mount point.");
-#if KDE_IS_VERSION(4,7,0)
-            reply.setErrorDescription(errorDescription);
-#else
-            reply.addData("errorDescription", errorDescription);
-#endif
+            reply.addData("errorDescription", i18nc("@info", "Failed to create \
temporary mount point."));  return reply;
         }
         ActionReply mountReply = executeCommand(QStringList() << "mount" << \
partition << mountPoint); @@ -209,16 +158,10 @@ ActionReply Helper::load(QVariantMap \
args)  QFile file(fileName);
     if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) {
         reply = ActionReply::HelperErrorReply;
-#if KDE_IS_VERSION(4,7,0)
-        reply.setErrorDescription(file.errorString());
-#else
         reply.addData("errorDescription", file.errorString());
-#endif
         return reply;
     }
-
-    QTextStream stream(&file);
-    reply.addData("fileContents", stream.readAll());
+    reply.addData("rawFileContents", file.readAll());
     return reply;
 }
 ActionReply Helper::probe(QVariantMap args)
@@ -271,9 +214,9 @@ ActionReply Helper::save(QVariantMap args)
     QString mkconfigExePath = args.value("mkconfigExePath").toString();
     QString set_defaultExePath = args.value("set_defaultExePath").toString();
     QString configFileName = args.value("configFileName").toString();
-    QString configFileContents = args.value("configFileContents").toString();
+    QByteArray rawConfigFileContents = \
args.value("rawConfigFileContents").toByteArray();  QString menuFileName = \
                args.value("menuFileName").toString();
-    QString defaultEntry = args.value("defaultEntry").toString();
+    QByteArray rawDefaultEntry = args.value("rawDefaultEntry").toByteArray();
     QString memtestFileName = args.value("memtestFileName").toString();
     bool memtest = args.value("memtest").toBool();
 
@@ -282,15 +225,10 @@ ActionReply Helper::save(QVariantMap args)
     QFile file(configFileName);
     if (!file.open(QIODevice::WriteOnly | QIODevice::Text)) {
         reply = ActionReply::HelperErrorReply;
-#if KDE_IS_VERSION(4,7,0)
-        reply.setErrorDescription(file.errorString());
-#else
         reply.addData("errorDescription", file.errorString());
-#endif
         return reply;
     }
-    QTextStream stream(&file);
-    stream << configFileContents;
+    file.write(rawConfigFileContents);
     file.close();
 
     if (args.contains("memtest")) {
@@ -308,7 +246,7 @@ ActionReply Helper::save(QVariantMap args)
         return grub_mkconfigReply;
     }
 
-    ActionReply grub_set_defaultReply = executeCommand(QStringList() << \
set_defaultExePath << defaultEntry); +    ActionReply grub_set_defaultReply = \
executeCommand(QStringList() << set_defaultExePath << rawDefaultEntry);  if \
(grub_set_defaultReply.failed()) {  return grub_set_defaultReply;
     }
diff --git a/src/kcm_grub2.cpp b/src/kcm_grub2.cpp
index 8dd14d6..5154df0 100644
--- a/src/kcm_grub2.cpp
+++ b/src/kcm_grub2.cpp
@@ -87,12 +87,13 @@ void KCMGRUB2::defaults()
 #endif
 
     ActionReply reply = defaultsAction.execute();
+    processReply(reply);
     if (reply.succeeded()) {
         load();
         save();
         KMessageBox::information(this, i18nc("@info", "Successfully restored the \
default values."));  } else {
-        KMessageBox::detailedError(this, i18nc("@info", "Failed to restore the \
default values."), KDE_IS_VERSION(4,7,0) ? reply.errorDescription() : \
reply.data().value("errorDescription").toString()); +        \
KMessageBox::detailedError(this, i18nc("@info", "Failed to restore the default \
values."), reply.errorDescription());  }
 }
 void KCMGRUB2::load()
@@ -414,9 +415,9 @@ void KCMGRUB2::save()
     saveAction.addArgument("mkconfigExePath", mkconfigExePath);
     saveAction.addArgument("set_defaultExePath", set_defaultExePath);
     saveAction.addArgument("configFileName", configPath);
-    saveAction.addArgument("configFileContents", configFileContents);
+    saveAction.addArgument("rawConfigFileContents", \
configFileContents.toLocal8Bit());  saveAction.addArgument("menuFileName", menuPath);
-    saveAction.addArgument("defaultEntry", m_entries.size() > 0 ? \
ui->kcombobox_default->currentText() : m_settings.value("GRUB_DEFAULT")); +    \
saveAction.addArgument("rawDefaultEntry", m_entries.size() > 0 ? \
ui->kcombobox_default->currentText().toLocal8Bit() : \
m_settings.value("GRUB_DEFAULT").toLocal8Bit());  if \
(m_dirtyBits.testBit(memtestDirty)) {  saveAction.addArgument("memtestFileName", \
                memtestPath);
         saveAction.addArgument("memtest", ui->checkBox_memtest->isChecked());
@@ -438,6 +439,7 @@ void KCMGRUB2::save()
     connect(saveAction.watcher(), SIGNAL(actionPerformed(ActionReply)), \
&progressDlg, SLOT(hide()));  
     ActionReply reply = saveAction.execute();
+    processReply(reply);
     if (reply.succeeded()) {
         KDialog *dialog = new KDialog(this, Qt::Dialog);
         dialog->setCaption(i18nc("@title:window", "Information"));
@@ -448,7 +450,7 @@ void KCMGRUB2::save()
         KMessageBox::createKMessageBox(dialog, QMessageBox::Information, \
i18nc("@info", "Successfully saved GRUB settings."), QStringList(), QString(), 0, \
KMessageBox::Notify, reply.data().value("output").toString()); // \
krazy:exclude=qclasses  load();
     } else {
-        KMessageBox::detailedError(this, i18nc("@info", "Failed to save GRUB \
settings."), KDE_IS_VERSION(4,7,0) ? reply.errorDescription() : \
reply.data().value("errorDescription").toString()); +        \
KMessageBox::detailedError(this, i18nc("@info", "Failed to save GRUB settings."), \
reply.errorDescription());  }
 }
 
@@ -972,12 +974,13 @@ QString KCMGRUB2::readFile(const QString &fileName)
 #endif
 
     ActionReply reply = loadAction.execute();
+    processReply(reply);
     if (reply.failed()) {
         kError() << "Error reading:" << fileName;
-        kError() << "Error description:" << (KDE_IS_VERSION(4,7,0) ? \
reply.errorDescription() : reply.data().value("errorDescription").toString()); +      \
kError() << "Error description:" << reply.errorDescription();  return QString();
     }
-    return reply.data().value("fileContents").toString();
+    return QString::fromLocal8Bit(reply.data().value("rawFileContents").toByteArray());
  }
 void KCMGRUB2::readEntries()
 {
@@ -1028,8 +1031,9 @@ void KCMGRUB2::readDevices()
     connect(probeAction.watcher(), SIGNAL(progressStep(int)), \
progressDlg.progressBar(), SLOT(setValue(int)));  
     ActionReply reply = probeAction.execute();
+    processReply(reply);
     if (reply.failed()) {
-        KMessageBox::detailedError(this, i18nc("@info", "Failed to get GRUB device \
names."), KDE_IS_VERSION(4,7,0) ? reply.errorDescription() : \
reply.data().value("errorDescription").toString()); +        \
KMessageBox::detailedError(this, i18nc("@info", "Failed to get GRUB device names."), \
reply.errorDescription());  return;
     }
     QStringList grubPartitions = \
reply.data().value("grubPartitions").toStringList(); @@ -1052,6 +1056,7 @@ void \
KCMGRUB2::readResolutions()  #endif
 
     ActionReply reply = probeVbeAction.execute();
+    processReply(reply);
     if (reply.failed()) {
         return;
     }
@@ -1110,7 +1115,7 @@ QString KCMGRUB2::unquoteWord(const QString &word)
     echo.setShellCommand(QString("echo -n %1").arg(word));
     echo.setOutputChannelMode(KProcess::OnlyStdoutChannel);
     if (echo.execute() == 0) {
-        return QString::fromUtf8(echo.readAllStandardOutput());
+        return QString::fromLocal8Bit(echo.readAllStandardOutput());
     }
 
     QChar ch;
@@ -1189,6 +1194,36 @@ QString KCMGRUB2::unquoteWord(const QString &word)
     return QString();
 }
 
+void KCMGRUB2::processReply(KAuth::ActionReply &reply)
+{
+    if (reply.type() == ActionReply::Success || reply.type() == \
ActionReply::KAuthError) { +        return;
+    }
+
+    if (reply.errorCode() == 0) {
+        QLatin1String key("errorDescription");
+        if (reply.data().contains(key)) {
+            reply.setErrorDescription(reply.data().value(key).toString());
+            reply.data().remove(key);
+        }
+        return;
+    }
+
+    QString errorMessage;
+    switch (reply.errorCode()) {
+    case -2:
+        errorMessage = i18nc("@info", "The process could not be started.");
+        break;
+    case -1:
+        errorMessage = i18nc("@info", "The process crashed.");
+        break;
+    default:
+        errorMessage = \
QString::fromLocal8Bit(reply.data().value(QLatin1String("output")).toByteArray()); +  \
break; +    }
+    reply.addData(QLatin1String("errorMessage"), errorMessage);
+    reply.setErrorDescription(i18nc("@info", "Command: \
<command>%1</command><nl/>Error code: <numid>%2</numid><nl/>Error \
message:<nl/><message>%3</message>", \
reply.data().value(QLatin1String("command")).toStringList().join(QLatin1String(" ")), \
reply.errorCode(), errorMessage)); +}
 void KCMGRUB2::parseEntries(const QString &config)
 {
     QChar ch;
diff --git a/src/kcm_grub2.h b/src/kcm_grub2.h
index 4a7a512..42dc721 100644
--- a/src/kcm_grub2.h
+++ b/src/kcm_grub2.h
@@ -23,6 +23,10 @@
 
 //KDE
 #include <KCModule>
+namespace KAuth
+{
+    class ActionReply;
+}
 
 //Ui
 namespace Ui
@@ -94,6 +98,7 @@ private:
     QString quoteWord(const QString &word);
     QString unquoteWord(const QString &word);
 
+    void processReply(KAuth::ActionReply &reply);
     void parseEntries(const QString &config);
     void parseSettings(const QString &config);
     void parseEnv(const QString &config);


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic