itemsync: Fix updating items in-place (#2459)

Fixes creating duplicate item clones when adding a tag for example. Fixes #2355
hluk · Sep 18, 2023 · 2642d71 · 2642d71
1 parent fbca952
commit 2642d71
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 22 deletions.
diff --git a/docs/scripting-api.rst b/docs/scripting-api.rst
@@ -1775,7 +1775,7 @@ Types
        for (var index = 0; index < sel.length; ++index) {
            var item = sel.itemAtIndex(index);
            item[mimeItemNotes] = 'Contains needle';
-           sel.setItemAtIndex(item);
+           sel.setItemAtIndex(index, item);
        }
 
    Example - selection with new items only:

diff --git a/plugins/itemsync/filewatcher.cpp b/plugins/itemsync/filewatcher.cpp
@@ -452,7 +452,7 @@ FileWatcher::FileWatcher(
              this, &FileWatcher::onDataChanged );
 
     if (model->rowCount() > 0)
-        saveItems(0, model->rowCount() - 1);
+        saveItems(0, model->rowCount() - 1, UpdateType::Inserted);
 
     prependItemsFromFiles( QDir(path), listFiles(paths, m_formatSettings, m_maxItems) );
 }
@@ -657,12 +657,12 @@ void FileWatcher::setUpdatesEnabled(bool enabled)
 
 void FileWatcher::onRowsInserted(const QModelIndex &, int first, int last)
 {
-    saveItems(first, last);
+    saveItems(first, last, UpdateType::Inserted);
 }
 
 void FileWatcher::onDataChanged(const QModelIndex &a, const QModelIndex &b)
 {
-    saveItems(a.row(), b.row());
+    saveItems(a.row(), b.row(), UpdateType::Changed);
 }
 
 void FileWatcher::onRowsRemoved(const QModelIndex &, int first, int last)
@@ -780,7 +780,7 @@ QList<QPersistentModelIndex> FileWatcher::indexList(int first, int last)
     return indexList;
 }
 
-void FileWatcher::saveItems(int first, int last)
+void FileWatcher::saveItems(int first, int last, UpdateType updateType)
 {
     if ( !lock() )
         return;
@@ -799,7 +799,7 @@ void FileWatcher::saveItems(int first, int last)
         return;
     }
 
-    if ( !renameMoveCopy(dir, indexList) )
+    if ( !renameMoveCopy(dir, indexList, updateType) )
         return;
 
     QStringList existingFiles = listFiles(dir);
@@ -877,19 +877,23 @@ void FileWatcher::saveItems(int first, int last)
     unlock();
 }
 
-bool FileWatcher::renameMoveCopy(const QDir &dir, const QList<QPersistentModelIndex> &indexList)
+bool FileWatcher::renameMoveCopy(
+    const QDir &dir, const QList<QPersistentModelIndex> &indexList, UpdateType updateType)
 {
     QStringList baseNames;
 
     for (const auto &index : indexList) {
         if ( !index.isValid() )
             continue;
 
-        const QString olderBaseName = oldBaseName(index);
+        QString olderBaseName = oldBaseName(index);
         const QString oldBaseName = getBaseName(index);
+        if (updateType == UpdateType::Changed && olderBaseName.isEmpty())
+            olderBaseName = oldBaseName;
+
         QString baseName = oldBaseName;
 
-        bool newItem = olderBaseName.isEmpty();
+        bool newItem = updateType != UpdateType::Changed && olderBaseName.isEmpty();
         bool itemRenamed = olderBaseName != baseName;
         if ( newItem || itemRenamed ) {
             if ( !renameToUnique(dir, baseNames, &baseName, m_formatSettings) )

diff --git a/plugins/itemsync/filewatcher.h b/plugins/itemsync/filewatcher.h
@@ -30,6 +30,11 @@ extern const QLatin1String mimePrivatePrefix;
 extern const QLatin1String mimeOldBaseName;
 extern const QLatin1String mimeHashPrefix;
 
+enum class UpdateType {
+    Inserted,
+    Changed,
+};
+
 struct FileFormat {
     bool isValid() const { return !extensions.isEmpty(); }
     QStringList extensions;
@@ -98,9 +103,10 @@ class FileWatcher final : public QObject {
 
     QList<QPersistentModelIndex> indexList(int first, int last);
 
-    void saveItems(int first, int last);
+    void saveItems(int first, int last, UpdateType updateType);
 
-    bool renameMoveCopy(const QDir &dir, const QList<QPersistentModelIndex> &indexList);
+    bool renameMoveCopy(
+        const QDir &dir, const QList<QPersistentModelIndex> &indexList, UpdateType updateType);
 
     void updateDataAndWatchFile(
             const QDir &dir, const BaseNameExtensions &baseNameWithExts,

diff --git a/plugins/itemsync/tests/itemsynctests.cpp b/plugins/itemsync/tests/itemsynctests.cpp
@@ -36,11 +36,8 @@ class TestDir final {
 
     void clear()
     {
-        if (isValid()) {
-            for ( const auto &fileName : files() )
-                remove(fileName);
-            m_dir.rmpath(".");
-        }
+        if (isValid())
+            m_dir.removeRecursively();
     }
 
     void create()
@@ -128,13 +125,9 @@ void ItemSyncTests::init()
 {
     TEST(m_test->init());
 
-    // Remove temporary directory.
-    for (int i = 0; i < 10; ++i)
-        TestDir(i, false);
-
     QDir tmpDir(QDir::cleanPath(testDir(0) + "/.."));
     if ( tmpDir.exists() )
-        QVERIFY(tmpDir.rmdir("."));
+        QVERIFY(tmpDir.removeRecursively());
 }
 
 void ItemSyncTests::cleanup()
@@ -389,12 +382,36 @@ void ItemSyncTests::modifyItems()
 
     RUN(args << "keys" << "HOME" << "DOWN" << "F2" << ":XXX" << "F2", "");
     RUN(args << "size", "4\n");
-    RUN(args << "read" << "0" << "1" << "2" << "3", "D,XXX,B,A");
+    RUN(args << "read" << "0" << "1" << "2" << "3" << "4", "D,XXX,B,A,");
 
     file = dir1.file(files[2]);
     QVERIFY(file->open(QIODevice::ReadOnly));
     QCOMPARE(file->readAll().data(), QByteArray("XXX").data());
     file->close();
+
+    const auto script = R"(
+        setCommands([{
+            name: 'Modify current item',
+            inMenu: true,
+            shortcuts: ['Ctrl+F1'],
+            cmd: `
+                copyq: item = selectedItemsData()[0]
+                item[mimeText] = "ZZZ"
+                setSelectedItemData(0, item)
+            `
+        }])
+        )";
+    RUN(script, "");
+    RUN(args << "keys" << "HOME" << "DOWN" << "CTRL+F1", "");
+    WAIT_ON_OUTPUT(args << "read" << "1", "ZZZ");
+    RUN(args << "read" << "0" << "1" << "2" << "3" << "4", "D,ZZZ,B,A,");
+    RUN(args << "unload" << tab1, "");
+    RUN(args << "read" << "0" << "1" << "2" << "3" << "4", "D,ZZZ,B,A,");
+
+    file = dir1.file(files[2]);
+    QVERIFY(file->open(QIODevice::ReadOnly));
+    QCOMPARE(file->readAll().data(), QByteArray("ZZZ").data());
+    file->close();
 }
 
 void ItemSyncTests::modifyFiles()