altered behavior of the 'move' operation

in case rename(...) call is not used, remove all files after everything
is copied (instead of removing each right after it was copied) and don't
remove at all if the target was present and not overwritten (temporary,
I'm not sure of the expected behavior in this case)

Signed-off-by: Lukas Hrazky <lukkash@email.cz>

diff --git a/src/fileoperator.cpp b/src/fileoperator.cpp
index 68ca019..8d422e6 100644 (file)
--- a/src/fileoperator.cpp
+++ b/src/fileoperator.cpp
@@ -28,6 +28,9 @@
 #include <iostream>
 
 
+#define BLOCK_SIZE 524288
+
+
 #define SHOW_ERROR_PROMPT(promptString, fileName)                                           \
     response = FileOperator::NONE;                                                          \
     if (ignoreAll[errno]) {                                                                 \
@@ -317,6 +320,10 @@ FileManipulatorThread::FileManipulatorThread(const QFileInfoList files, QDir des
 
 
 FileManipulatorThread::~FileManipulatorThread() {
+    if (progressBar->value() < progressBar->maximum()) {
+        std::cout << "WARNING: deleting a progressbar which's value " << progressBar->value() <<
+            " has not reached maximum of " << progressBar->maximum() << std::endl;
+    }
     delete progressBar;
 }
 
@@ -358,46 +365,48 @@ void FileManipulatorThread::processFiles(const QFileInfoList &files) {
 }
 
 
-bool FileManipulatorThread::remove(QString &fileName, const bool ignoreDirNotEmpty) {
-    return remove(QFileInfo(fileName), ignoreDirNotEmpty);
+bool FileManipulatorThread::remove(QString &fileName, const bool doUpdates) {
+    return remove(QFileInfo(fileName), doUpdates);
 }
 
 
-bool FileManipulatorThread::remove(const QFileInfoList &files, const bool ignoreDirNotEmpty) {
+bool FileManipulatorThread::remove(const QFileInfoList &files, const bool doUpdates) {
     bool res = true;
     for (QFileInfoList::const_iterator it = files.begin(); it != files.end(); ++it) {
-        if (!remove(*it, ignoreDirNotEmpty)) res = false;
+        if (!remove(*it, doUpdates)) res = false;
         if (abort) break;
     }
     return res;
 }
 
 
-bool FileManipulatorThread::remove(const QFileInfo &file, const bool ignoreDirNotEmpty) {
+bool FileManipulatorThread::remove(const QFileInfo &file, const bool doUpdates) {
+    std::cout << "DELETING " << file.absoluteFilePath().toStdString() << std::endl;
+
     QString path = file.absoluteFilePath();
     QFSFileEngine engine(path);
 
-    if (file.isDir()) {
-        QFileInfoList list = listDirFiles(path);
+    if (doUpdates) updateFile(path);
 
-        if (ignoreDirNotEmpty && list.size()) return true;
-
-        if (!remove(list, ignoreDirNotEmpty)) return false;
+    if (file.isDir()) {
+        if (!remove(listDirFiles(path), doUpdates)) return false;
 
-        ERROR_PROMPT(!engine.rmdir(path, false),
-            tr("Error deleting directory %1."), path)
+        if (!listDirFiles(path).size()) {
+            ERROR_PROMPT(!engine.rmdir(path, false), tr("Error deleting directory %1."), path)
+        }
     } else {
-        ERROR_PROMPT(!engine.remove(),
-            tr("Error deleting file %1."), path)
+        ERROR_PROMPT(!engine.remove(), tr("Error deleting file %1."), path)
     }
 
+    if (!abort && doUpdates) updateProgress(1);
+
     if (abort || response == FileOperator::IGNORE) return false;
     return true;
 }
 
 
-void FileManipulatorThread::copy(const QFileInfo &file, const bool removeAfterCopy) {
-    std::cout << (removeAfterCopy ? "MOVING " : "COPYING ") << file.absoluteFilePath().toStdString()
+void FileManipulatorThread::copy(const QFileInfo &file) {
+    std::cout << "COPYING " << file.absoluteFilePath().toStdString()
         << " to " << dest.absolutePath().toStdString() << std::endl;
 
     QString path(file.absoluteFilePath());
@@ -417,161 +426,149 @@ void FileManipulatorThread::copy(const QFileInfo &file, const bool removeAfterCo
 
     if (abort) return;
 
-    // this loop is here only to allow easily breaking out to the end (and remove the source file/dir)
-    while (1) {
-        if (response == FileOperator::KEEP) {
-            updateProgress(fileSizeMap[path]);
-            break;
-        }
+    if (response == FileOperator::KEEP) {
+        updateProgress(fileSizeMap[path]);
+        return;
+    }
 
+    if (file.isDir()) {
         FileOperator::Response overwriteResponse = response;
 
-        if (file.isDir()) {
-            if (newFile.exists() && !newFile.isDir()) {
-                if(!remove(newPath)) {
-                    updateProgress(fileSizeMap[path]);
-                    break;
-                }
-                newFile = QFileInfo(newPath);
+        if (newFile.exists() && !newFile.isDir()) {
+            if(!remove(newPath)) {
+                updateProgress(fileSizeMap[path]);
+                return;
             }
+            newFile = QFileInfo(newPath);
+        }
 
-            if (!newFile.exists()) {
-                ERROR_PROMPT_XP(!engine.mkdir(newPath, false),
-                    tr("Error creating directory %1."), newPath,
-                    updateProgress(fileSizeMap[path]),
-                    break)
-            }
+        if (!newFile.exists()) {
+            ERROR_PROMPT_XP(!engine.mkdir(newPath, false),
+                tr("Error creating directory %1."), newPath,
+                updateProgress(fileSizeMap[path]),
+                return)
+        }
 
-            updateProgress(1);
-            
-            QDir destBackup = dest;
-            dest = newPath;
+        updateProgress(1);
+        
+        QDir destBackup = dest;
+        dest = newPath;
 
-            FileOperator::Response tmpResp = overwriteAll;
-            overwriteAll = overwriteResponse;
+        FileOperator::Response tmpResp = overwriteAll;
+        overwriteAll = overwriteResponse;
 
-            processFiles(listDirFiles(path));
+        processFiles(listDirFiles(path));
 
-            overwriteAll = tmpResp;
+        overwriteAll = tmpResp;
 
-            ERROR_PROMPT(!newEngine.setPermissions(file.permissions()),
-                tr("Error setting permissions for directory %1."), newPath)
+        ERROR_PROMPT(!newEngine.setPermissions(file.permissions()),
+            tr("Error setting permissions for directory %1."), newPath)
 
-            if (abort) return;
+        if (abort) return;
 
-            dest = destBackup;
-        } else {
-            ERROR_PROMPT_XP(engine.isSequential(),
-                tr("Cannot copy sequential file %1."), path,
+        dest = destBackup;
+    } else {
+        ERROR_PROMPT_XP(engine.isSequential(),
+            tr("Cannot copy sequential file %1."), path,
+            updateProgress(fileSizeMap[path]),
+            return)
+
+        if (newFile.exists() && newFile.isDir()) {
+            ERROR_PROMPT_XP(!remove(newPath),
+                tr("Cannot replace directory %1 due to previous errors."), newPath,
                 updateProgress(fileSizeMap[path]),
-                break)
+                return)
+        }
 
-            if (newFile.exists() && newFile.isDir()) {
-                ERROR_PROMPT_XP(!remove(newPath),
-                    tr("Cannot replace directory %1 due to previous errors."), newPath,
-                    updateProgress(fileSizeMap[path]),
-                    break)
-            }
+        ERROR_PROMPT_XP(!engine.open(QIODevice::ReadOnly),
+            tr("Error reading file %1."), path,
+            updateProgress(fileSizeMap[path]),
+            return)
 
-            ERROR_PROMPT_XP(!engine.open(QIODevice::ReadOnly),
-                tr("Error reading file %1."), path,
-                updateProgress(fileSizeMap[path]),
-                break)
+        bool ignore = false;
+        while (!abort && !ignore) {
+            engine.seek(0);
 
-            bool ignore = false;
-            while (!abort && !ignore) {
-                engine.seek(0);
+            ERROR_PROMPT(!newEngine.open(QIODevice::WriteOnly | QIODevice::Truncate),
+                tr("Error writing file %1."), newPath)
 
-                ERROR_PROMPT(!newEngine.open(QIODevice::WriteOnly | QIODevice::Truncate),
-                    tr("Error writing file %1."), newPath)
+            if (abort || response == FileOperator::IGNORE) {
+                if (response == FileOperator::IGNORE) {
+                    updateProgress(fileSizeMap[path] - fileValue);
+                    ignore = true;
+                }
+                break;
+            }
 
-                if (abort || response == FileOperator::IGNORE) {
-                    if (response == FileOperator::IGNORE) {
-                        updateProgress(fileSizeMap[path] - fileValue);
-                        ignore = true;
+            bool error = false;
+            char block[BLOCK_SIZE];
+            qint64 bytes;
+            while ((bytes = engine.read(block, sizeof(block))) > 0) {
+                if (bytes == -1 || bytes != newEngine.write(block, bytes)) {
+                    if (bytes == -1) {
+                        SHOW_ERROR_PROMPT(tr("Error while reading from file %1."), path);
+                    } else {
+                        SHOW_ERROR_PROMPT(tr("Error while writing to file %1."), newPath);
                     }
-                    break;
-                }
 
-                bool error = false;
-                char block[524288];
-                qint64 bytes;
-                while ((bytes = engine.read(block, sizeof(block))) > 0) {
-                    if (bytes == -1 || bytes != newEngine.write(block, bytes)) {
-                        if (bytes == -1) {
-                            SHOW_ERROR_PROMPT(tr("Error while reading from file %1."), path);
+                    if (!abort) {
+                        if (response == FileOperator::IGNORE) {
+                            updateProgress(fileSizeMap[path] - fileValue);
+                            ignore = true;
                         } else {
-                            SHOW_ERROR_PROMPT(tr("Error while writing to file %1."), newPath);
+                            updateProgress(-fileValue);
                         }
-
-                        if (!abort) {
-                            if (response == FileOperator::IGNORE) {
-                                updateProgress(fileSizeMap[path] - fileValue);
-                                ignore = true;
-                            } else {
-                                updateProgress(-fileValue);
-                            }
-                        }
-                        error = true;
-                        break;
                     }
-
-                    updateProgress(1);
+                    error = true;
+                    break;
                 }
 
-                if (!error) break;
+                updateProgress(1);
             }
 
-            engine.close();
-            newEngine.close();
-
-            if (abort || ignore) {
-                newEngine.remove();
-            } else {
-                ERROR_PROMPT(!newEngine.setPermissions(file.permissions()),
-                    tr("Error setting permissions for file %1."), newPath)
-            }
+            if (!error) break;
         }
 
-        break;
-    }
+        engine.close();
+        newEngine.close();
 
-    if (removeAfterCopy && !abort) remove(path, true);
+        if (abort || ignore) {
+            newEngine.remove();
+        } else {
+            ERROR_PROMPT(!newEngine.setPermissions(file.permissions()),
+                tr("Error setting permissions for file %1."), newPath)
+        }
+    }
 }
 
 
-unsigned int FileManipulatorThread::countFiles(const QFileInfoList &files) {
+unsigned int FileManipulatorThread::calculateFileSize(const QFileInfoList &files,
+    const bool count,
+    const bool addSize)
+{
     unsigned int res = 0;
 
     for (QFileInfoList::const_iterator it = files.begin(); it != files.end(); ++it) {
-        unsigned int size = 1;
+        unsigned int size = 0;
 
         if (it->isDir()) {
-            size += countFiles(listDirFiles(it->absoluteFilePath()));
+            size += calculateFileSize(listDirFiles(it->absoluteFilePath()), count, addSize);
         }
 
-        res += size;
-        fileSizeMap[it->absoluteFilePath()] = size;
-    }
-
-    return res;
-}
-
-
-unsigned int FileManipulatorThread::calculateFileSize(const QFileInfoList &files) {
-    unsigned int res = 0;
-
-    for (QFileInfoList::const_iterator it = files.begin(); it != files.end(); ++it) {
-        unsigned int size = 1;
+        if (addSize) {
+            if (it->isDir()) {
+                ++size;
+            } else {
+                size += ceil(static_cast<float>(it->size()) / BLOCK_SIZE);
+            }
+            fileSizeMap[it->absoluteFilePath()] = size;
+        }
 
-        if (it->isDir()) {
-            size += calculateFileSize(listDirFiles(it->absoluteFilePath()));
-        } else {
-            size = ceil(static_cast<float>(it->size()) / 524288);
+        if (count) {
+            ++size;
         }
 
         res += size;
-        fileSizeMap[it->absoluteFilePath()] = size;
     }
 
     return res;
@@ -606,7 +603,8 @@ void FileManipulatorThread::updateFile(const QString &name) {
 
 void FileManipulatorThread::setText(int value) {
     if (progressBar->value() + value > progressBar->maximum()) {
-        std::cout << "WARNING, EXCEEDING MAXIMUM BY " << value << std::endl;
+        std::cout << "WARNING: exceeding progressbar maximum (" << progressBar->maximum()
+            << ") by " << value << std::endl;
     }
  
     time_t now = time(0);
@@ -639,7 +637,7 @@ DeleteThread::DeleteThread(const QFileInfoList &files) : FileManipulatorThread(f
 void DeleteThread::run() {
     mutex.lock();
 
-    setBarSize(countFiles(files));
+    setBarSize(calculateFileSize(files, true));
 
     processFiles(files);
 
@@ -649,26 +647,7 @@ void DeleteThread::run() {
 
 
 void DeleteThread::perform(const QFileInfo &file) {
-    std::cout << "DELETING " << file.absoluteFilePath().toStdString() << std::endl;
-
-    QString path = file.absoluteFilePath();
-    QFSFileEngine engine(path);
-
-    updateFile(path);
-
-    if (file.isDir()) {
-        processFiles(listDirFiles(path));
-
-        if (!listDirFiles(path).size()) {
-            ERROR_PROMPT(!engine.rmdir(path, false),
-                tr("Error deleting directory %1."), path)
-        }
-    } else {
-        ERROR_PROMPT(!engine.remove(),
-            tr("Error deleting file %1."), path)
-    }
-
-    if (!abort) updateProgress(1);
+    remove(file, true);
 }
 
 
@@ -680,7 +659,7 @@ CopyThread::CopyThread(const QFileInfoList &files, QDir &dest) : FileManipulator
 void CopyThread::run() {
     mutex.lock();
 
-    setBarSize(calculateFileSize(files));
+    setBarSize(calculateFileSize(files, false, true));
 
     processFiles(files);
 
@@ -690,7 +669,7 @@ void CopyThread::run() {
 
 
 void CopyThread::perform(const QFileInfo &file) {
-    copy(file, false);
+    copy(file);
 }
 
 
@@ -722,7 +701,8 @@ void MoveThread::rename(const QFileInfoList &files, const QDir &dest) {
         OVERWRITE_PROMPT(files[i], QFileInfo(newPath))
 
         if (response == FileOperator::KEEP) {
-            remove(path);
+            // TODO lets not remove the source for now, I'm not sure what is correct behavior
+            // remove(path);
             if (abort) break;
             updateProgress(1);
             continue;
@@ -733,17 +713,20 @@ void MoveThread::rename(const QFileInfoList &files, const QDir &dest) {
             // this should happen on the first file, unless some are skipped by overwrite prompt
             // we calculate the actual file sizes, because from now on copy & remove takes over
             if (errno == EXDEV) {
-                setBarSize(barValue + calculateFileSize(files));
-
-                FileOperator::Response tmpResp = overwriteAll;
                 overwriteAll = response;
                 // hack: we already checked the first file we are sending to processFiles(...)
                 // so we don't want to ask about this one again
                 if (overwriteAll == FileOperator::NONE) overwriteAll = FileOperator::DONT_ASK_ONCE;
 
-                processFiles(files.mid(i));
+                QFileInfoList remainingFiles = files.mid(i);
 
-                overwriteAll = tmpResp;
+                setBarSize(barValue + calculateFileSize(remainingFiles, true, true));
+
+                processFiles(remainingFiles);
+
+                barText = tr("deleting %1");
+
+                remove(remainingFiles, true);
 
                 // just to quit the loops, we are done
                 abort = true;
@@ -780,5 +763,5 @@ void MoveThread::rename(const QFileInfoList &files, const QDir &dest) {
 
 
 void MoveThread::perform(const QFileInfo &file) {
-    copy(file, true);
+    copy(file);
 }

diff --git a/src/fileoperator.h b/src/fileoperator.h
index 9e2dcfb..1604b9e 100644 (file)
--- a/src/fileoperator.h
+++ b/src/fileoperator.h
@@ -84,14 +84,15 @@ protected:
     void processFiles(const QFileInfoList &files);
     virtual void perform(const QFileInfo &file) = 0;
 
-    bool remove(QString &fileName, const bool ignoreDirNotEmpty = false);
-    bool remove(const QFileInfoList &files, const bool ignoreDirNotEmpty = false);
-    bool remove(const QFileInfo &file, const bool ignoreDirNotEmpty = false);
+    bool remove(QString &fileName, const bool doUpdates = false);
+    bool remove(const QFileInfoList &files, const bool doUpdates = false);
+    bool remove(const QFileInfo &file, const bool doUpdates = false);
 
-    void copy(const QFileInfo &file, const bool removeAfterCopy);
+    void copy(const QFileInfo &file);
 
-    unsigned int countFiles(const QFileInfoList &files);
-    unsigned int calculateFileSize(const QFileInfoList &files);
+    unsigned int calculateFileSize(const QFileInfoList &files,
+        const bool count = false,
+        const bool addSize = false);
 
     QFileInfoList listDirFiles(const QString &dirPath);