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

List:       kde-commits
Subject:    Re: [ark/frameworks] /: Implement a cancelled() signal for CliInterface
From:       Raphael Kubo da Costa <rakuco () FreeBSD ! org>
Date:       2015-05-31 16:34:25
Message-ID: 86r3pwbt9q.fsf () FreeBSD ! org
[Download RAW message or body]

Raphael Kubo da Costa <rakuco@FreeBSD.org> writes:

> My suggestion: revert this commit, change the finished(bool) signal in
> ReadOnlyArchiveInterface to finished(ExitStatus), where ExitStatus is an
> enum with two values so far, Success and Cancelled.

... though admittedly this might be tricky for cases where there's a
process running since killing it will cause the finished signal to be
emitted too.

Unfortunately the interaction between ArchiveInterface and Job is quite
messy and should be cleaned up. With that in mind, the easiest way to
achieve your original goal without getting tangled up in this problem is
to keep the cancelled() signal in ReadOnlyArchiveInterface but make it
have a different effect in Job.

Something like the patch I'm attaching (it was made against master, but
it should not be difficult to make it apply to the frameworks branch).
In this case, I don't think you need to revert your commit anymore, but
I'd like to take a look at the patch you come up with before you land
it.


["cancelled-signal-generates-error.patch" (text/x-patch)]

diff --git a/app/batchextract.cpp b/app/batchextract.cpp
index 217badd..0edd1ab 100644
--- a/app/batchextract.cpp
+++ b/app/batchextract.cpp
@@ -159,7 +159,7 @@ void BatchExtract::slotResult(KJob *job)
     // TODO: The user must be informed about which file caused the error, and that \
the other files  //       in the queue will not be extracted.
     if (job->error()) {
-        kDebug() << "There was en error, " << job->errorText();
+        kDebug() << "There was an error:" << job->errorText();
 
         setErrorText(job->errorText());
         setError(job->error());
diff --git a/kerfuffle/archiveinterface.h b/kerfuffle/archiveinterface.h
index 801318b..220909d 100644
--- a/kerfuffle/archiveinterface.h
+++ b/kerfuffle/archiveinterface.h
@@ -94,6 +94,7 @@ public:
     virtual bool doResume();
 
 signals:
+    void cancelled();
     void error(const QString &message, const QString &details = QString());
     void entry(const ArchiveEntry &archiveEntry);
     void entryRemoved(const QString &path);
diff --git a/kerfuffle/cliinterface.cpp b/kerfuffle/cliinterface.cpp
index eab1c6a..929a809 100644
--- a/kerfuffle/cliinterface.cpp
+++ b/kerfuffle/cliinterface.cpp
@@ -161,6 +161,9 @@ bool CliInterface::copyFiles(const QList<QVariant> & files, const \
QString & dest  query.waitForResponse();
 
                 if (query.responseCancelled()) {
+                    emit cancelled();
+                    // There is no process running, so finished() must be emitted \
manually. +                    emit finished(false);
                     failOperation();
                     return false;
                 }
@@ -516,6 +519,7 @@ void CliInterface::handleLine(const QString& line)
             query.waitForResponse();
 
             if (query.responseCancelled()) {
+                emit cancelled();
                 failOperation();
                 return;
             }
@@ -556,6 +560,7 @@ void CliInterface::handleLine(const QString& line)
             query.waitForResponse();
 
             if (query.responseCancelled()) {
+                emit cancelled();
                 failOperation();
                 return;
             }
diff --git a/kerfuffle/jobs.cpp b/kerfuffle/jobs.cpp
index 472a6e0..411cb38 100644
--- a/kerfuffle/jobs.cpp
+++ b/kerfuffle/jobs.cpp
@@ -114,6 +114,7 @@ void Job::emitResult()
 
 void Job::connectToArchiveInterfaceSignals()
 {
+    connect(archiveInterface(), SIGNAL(cancelled()), SLOT(onCancelled()));
     connect(archiveInterface(), SIGNAL(error(QString,QString)), \
                SLOT(onError(QString,QString)));
     connect(archiveInterface(), SIGNAL(entry(ArchiveEntry)), \
                SLOT(onEntry(ArchiveEntry)));
     connect(archiveInterface(), SIGNAL(entryRemoved(QString)), \
SLOT(onEntryRemoved(QString))); @@ -123,6 +124,11 @@ void \
                Job::connectToArchiveInterfaceSignals()
     connect(archiveInterface(), SIGNAL(userQuery(Query*)), \
SLOT(onUserQuery(Query*)));  }
 
+void Job::onCancelled()
+{
+    setError(KJob::KilledJobError);
+}
+
 void Job::onError(const QString & message, const QString & details)
 {
     Q_UNUSED(details)
diff --git a/kerfuffle/jobs.h b/kerfuffle/jobs.h
index d704dfc..a0501f8 100644
--- a/kerfuffle/jobs.h
+++ b/kerfuffle/jobs.h
@@ -66,6 +66,7 @@ public slots:
     virtual void doWork() = 0;
 
 protected slots:
+    virtual void onCancelled();
     virtual void onError(const QString &message, const QString &details);
     virtual void onInfo(const QString &info);
     virtual void onEntry(const ArchiveEntry &archiveEntry);
diff --git a/part/part.cpp b/part/part.cpp
index 517f8bb..c8bf010 100644
--- a/part/part.cpp
+++ b/part/part.cpp
@@ -501,7 +501,13 @@ void Part::slotLoadingFinished(KJob *job)
 
     if (job->error()) {
         if (arguments().metaData()[QLatin1String( "createNewArchive" )] != \
                QLatin1String( "true" )) {
-            KMessageBox::sorry(NULL, i18nc("@info", "Loading the archive \
<filename>%1</filename> failed with the following error: <message>%2</message>", \
localFilePath(), job->errorText()), i18nc("@title:window", "Error Opening Archive")); \
+            if (job->error() != KJob::KilledJobError) { +                \
KMessageBox::sorry( +                    NULL,
+                    i18nc("@info", "Loading the archive <filename>%1</filename> \
failed with the following error: <message>%2</message>", +                          \
localFilePath(), job->errorText()), +                    i18nc("@title:window", \
"Error Opening Archive")); +            }
 
             // The file failed to open, so reset the open archive, info panel and \
caption.  m_model->setArchive(NULL);
@@ -589,7 +595,9 @@ void Part::slotPreviewExtracted(KJob *job)
     //        if there's an error or an overwrite dialog,
     //        the preview dialog will be launched anyway
     if (job->error()) {
-        KMessageBox::error(widget(), job->errorString());
+        if (job->error() != KJob::KilledJobError) {
+            KMessageBox::error(widget(), job->errorString());
+        }
         return;
     }
 
@@ -743,7 +751,9 @@ void Part::slotExtractionDone(KJob* job)
 {
     kDebug();
     if (job->error()) {
-        KMessageBox::error(widget(), job->errorString());
+        if (job->error() != KJob::KilledJobError) {
+            KMessageBox::error(widget(), job->errorString());
+        }
     } else {
         ExtractJob *extractJob = qobject_cast<ExtractJob*>(job);
         Q_ASSERT(extractJob);
@@ -849,7 +859,7 @@ void Part::slotAddDir()
 void Part::slotAddFilesDone(KJob* job)
 {
     kDebug();
-    if (job->error()) {
+    if (job->error() && job->error() != KJob::KilledJobError) {
         KMessageBox::error(widget(), job->errorString());
     }
 }
@@ -857,7 +867,7 @@ void Part::slotAddFilesDone(KJob* job)
 void Part::slotDeleteFilesDone(KJob* job)
 {
     kDebug();
-    if (job->error()) {
+    if (job->error() && job->error() != KJob::KilledJobError) {
         KMessageBox::error(widget(), job->errorString());
     }
 }



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