Fix moving tab group with items in tree

This fixes moving "c" (tab with item) to "b" (group) in the following
tab tree:

    a
    └──b
       └──c
          └──d

The result should be:

    a
    └──c
       └──d

The problem was that "b" group was empty while iterating the tree, which
broke the iteration process. The fix is to delete all empty groups after
the iteration.

Author: hluk

src/gui/mainwindow.cpp

@@ -1530,13 +1530,6 @@ QAction *MainWindow::addTrayAction(Actions::Id id)
     return act;
 }
 
-void MainWindow::updateTabIcon(const QString &newName, const QString &oldName)
-{
-    const QString icon = getIconNameForTabName(oldName);
-    if ( !icon.isEmpty() )
-        setIconNameForTabName(newName, icon);
-}
-
 template <typename Receiver, typename ReturnType>
 QAction *MainWindow::addItemAction(Actions::Id id, Receiver *receiver, ReturnType (Receiver::* slot)())
 {
@@ -3134,24 +3127,25 @@ void MainWindow::doSaveTabPositions(AppConfig *appConfig)
 
 void MainWindow::tabsMoved(const QString &oldPrefix, const QString &newPrefix)
 {
-    const QStringList tabs = ui->tabWidget->tabs();
-    Q_ASSERT( oldPrefix == newPrefix || !tabs.contains(oldPrefix) );
-    Q_ASSERT( !tabs.contains(QString()) );
+    const QStringList newTabNames = ui->tabWidget->tabs();
+    Q_ASSERT( oldPrefix == newPrefix || !newTabNames.contains(oldPrefix) );
+    Q_ASSERT( !newTabNames.contains(QString()) );
 
-    const QString prefix = oldPrefix + '/';
+    AppConfig appConfig;
+    Tabs tabs;
 
     // Rename tabs if needed.
-    for (int i = 0 ; i < tabs.size(); ++i) {
+    for (int i = 0 ; i < newTabNames.size(); ++i) {
+        const QString &newTabName = newTabNames[i];
         auto placeholder = getPlaceholder(i);
         const QString oldTabName = placeholder->tabName();
-
-        if ( (oldTabName == oldPrefix || oldTabName.startsWith(prefix)) && newPrefix != oldPrefix) {
-            const QString newName = newPrefix + oldTabName.mid(oldPrefix.size());
-            setTabName(placeholder, newName);
-        }
+        if (newTabName != oldTabName)
+            updateTabName(placeholder, newTabName, &appConfig, &tabs);
     }
 
-    saveTabPositions();
+    const QStringList tabNames = ui->tabWidget->tabs();
+    tabs.save(&appConfig.settings(), tabNames);
+    appConfig.setOption(Config::tabs::name(), tabNames);
 }
 
 void MainWindow::tabBarMenuRequested(QPoint pos, int tab)
@@ -3480,31 +3474,34 @@ void MainWindow::onItemDoubleClicked()
         activateCurrentItem();
 }
 
-bool MainWindow::setTabName(ClipboardBrowserPlaceholder *placeholder, const QString &newName)
+bool MainWindow::updateTabName(
+    ClipboardBrowserPlaceholder *placeholder,
+    const QString &newName,
+    AppConfig *appConfig,
+    Tabs *tabs)
 {
     const QString oldName = placeholder->tabName();
     if ( !placeholder->setTabName(newName) )
         return false;
 
-    updateTabIcon(newName, oldName);
-    const int tabIndex = findTabIndex(oldName);
-    Q_ASSERT(tabIndex != -1);
-    ui->tabWidget->setTabName(tabIndex, newName);
-
-    Tabs tabs;
-    TabProperties tabProperties = tabs.tabProperties(oldName);
+    TabProperties tabProperties = tabs->tabProperties(oldName);
     tabProperties.name = newName;
-    tabs.setTabProperties(tabProperties);
-    Settings settings;
-    const QStringList tabNames = ui->tabWidget->tabs();
-    tabs.save(&settings, tabNames);
-    settings.setValue("Options/tabs", tabNames);
-
-    if (oldName == m_options.clipboardTab)
-        settings.setValue("Options/clipboard_tab", newName);
+    tabs->setTabProperties(tabProperties);
+
+    if (oldName == m_options.clipboardTab) {
+        m_options.clipboardTab = newName;
+        appConfig->setOption(Config::clipboard_tab::name(), newName);
+        // Restart clipboard monitoring to apply new clipboard tab configuration.
+        if (!m_clipboardStoringDisabled) {
+            emit disableClipboardStoring(true);
+            emit disableClipboardStoring(false);
+        }
+    }
 
-    if (oldName == m_options.trayTabName)
-        settings.setValue("Options/tray_tab", newName);
+    if (oldName == m_options.trayTabName) {
+        m_options.trayTabName = newName;
+        appConfig->setOption(Config::tray_tab::name(), newName);
+    }
 
     return true;
 }
@@ -4211,8 +4208,19 @@ void MainWindow::renameTab(const QString &name, int tabIndex)
         return;
 
     auto placeholder = getPlaceholder(tabIndex);
-    if (placeholder)
-        setTabName(placeholder, name);
+    if (!placeholder)
+        return;
+
+    AppConfig appConfig;
+    Tabs tabs;
+    if ( !updateTabName(placeholder, name, &appConfig, &tabs) )
+        return;
+
+    ui->tabWidget->setTabName(tabIndex, name);
+
+    const QStringList tabNames = ui->tabWidget->tabs();
+    tabs.save(&appConfig.settings(), tabNames);
+    appConfig.setOption(Config::tabs::name(), tabNames);
 }
 
 void MainWindow::removeTabGroup(const QString &name)

src/gui/mainwindow.h

@@ -430,6 +430,8 @@ class MainWindow final : public QMainWindow
 
     void sendActionData(int actionId, const QByteArray &bytes);
 
+    void clipboardTabChanged();
+
 protected:
     bool eventFilter(QObject *object, QEvent *ev) override;
     void keyPressEvent(QKeyEvent *event) override;
@@ -588,8 +590,6 @@ class MainWindow final : public QMainWindow
 
     QAction *addTrayAction(Actions::Id id);
 
-    void updateTabIcon(const QString &newName, const QString &oldName);
-
     template <typename Receiver, typename ReturnType>
     QAction *addItemAction(Actions::Id id, Receiver *receiver, ReturnType (Receiver::* slot)());
 
@@ -640,7 +640,16 @@ class MainWindow final : public QMainWindow
     void onItemClicked();
     void onItemDoubleClicked();
 
-    bool setTabName(ClipboardBrowserPlaceholder *placeholder, const QString &newName);
+    /**
+     * Update tab name in placeholder and configuration.
+     * Return true on success, false if setting tab name in placeholder failed
+     * (most likely failure to move the tab data).
+     */
+    bool updateTabName(
+        ClipboardBrowserPlaceholder *placeholder,
+        const QString &newName,
+        AppConfig *appConfig,
+        Tabs *tabs);
 
     ConfigurationManager *cm;
     Ui::MainWindow *ui;

src/gui/tabtree.cpp

@@ -591,11 +591,12 @@ void TabTree::dropEvent(QDropEvent *event)
 
         QList<QTreeWidgetItem*> newTabs;
         QList<int> indexes;
+        QList<QPersistentModelIndex> toDelete;
         for ( QTreeWidgetItemIterator it(topLevelItem(0)); *it; ++it ) {
             auto item = *it;
             // Remove empty groups.
             if ( isEmptyTabGroup(item) ) {
-                deleteItem(item);
+                toDelete.append(indexFromItem(item));
             } else {
                 const int oldIndex = getTabIndex(item);
                 if (oldIndex != -1) {
@@ -605,10 +606,19 @@ void TabTree::dropEvent(QDropEvent *event)
             }
         }
 
-        m_tabs = std::move(newTabs);
-        emit tabsMoved(oldPrefix, newPrefix, indexes);
+        for (const auto &index : toDelete) {
+            if (!index.isValid())
+                continue;
+
+            QTreeWidgetItem *item = itemFromIndex(index);
+            if (item)
+                deleteItem(item);
+        }
 
         updateSize();
+
+        m_tabs = std::move(newTabs);
+        emit tabsMoved(oldPrefix, newPrefix, indexes);
     } else {
         event->ignore();
     }