diff --git a/docs/scripting-api.rst b/docs/scripting-api.rst index 30859fe11d..8931d5c935 100644 --- a/docs/scripting-api.rst +++ b/docs/scripting-api.rst @@ -1637,9 +1637,13 @@ unlike in GUI, where row numbers start from 1 by default. The target tab is returned by `selectedTab()`. - The removed items can be accessed with `selectedItemsData()`, + The items scheduled for removal can be accessed with `selectedItemsData()`, `selectedItemData()`, `selectedItems()` and `ItemSelection().current()`. + If the exit code is non-zero (for example `fail()` is called), items will + not be removed. But this can also cause a new items not to be added if the + tab is full. + .. js:function:: onItemsChanged() Called when data in items change. diff --git a/plugins/itemsync/itemsync.cpp b/plugins/itemsync/itemsync.cpp index 9073b5ee8b..38ccd0489f 100644 --- a/plugins/itemsync/itemsync.cpp +++ b/plugins/itemsync/itemsync.cpp @@ -465,7 +465,7 @@ bool ItemSyncSaver::canRemoveItems(const QList &indexList, QString QMessageBox::Yes ) == QMessageBox::Yes; } -void ItemSyncSaver::itemsRemovedByUser(const QList &indexList) +void ItemSyncSaver::itemsRemovedByUser(const QList &indexList) { if ( m_tabPath.isEmpty() ) return; diff --git a/plugins/itemsync/itemsync.h b/plugins/itemsync/itemsync.h index 3178a7863f..bd32d0054e 100644 --- a/plugins/itemsync/itemsync.h +++ b/plugins/itemsync/itemsync.h @@ -50,7 +50,7 @@ class ItemSyncSaver final : public QObject, public ItemSaverInterface bool canRemoveItems(const QList &indexList, QString *error) override; - void itemsRemovedByUser(const QList &indexList) override; + void itemsRemovedByUser(const QList &indexList) override; QVariantMap copyItem(const QAbstractItemModel &model, const QVariantMap &itemData) override; diff --git a/src/gui/clipboardbrowser.cpp b/src/gui/clipboardbrowser.cpp index 61d97d16eb..05c75e2ae5 100644 --- a/src/gui/clipboardbrowser.cpp +++ b/src/gui/clipboardbrowser.cpp @@ -9,9 +9,6 @@ #include "common/temporaryfile.h" #include "common/textdata.h" #include "common/timer.h" -#include "gui/clipboarddialog.h" -#include "gui/iconfactory.h" -#include "gui/icons.h" #include "gui/pixelratio.h" #include "gui/theme.h" #include "item/itemeditor.h" @@ -541,17 +538,21 @@ void ClipboardBrowser::dropIndexes(const QModelIndexList &indexes) { auto toRemove = toPersistentModelIndexList(indexes); std::sort( std::begin(toRemove), std::end(toRemove) ); + dropIndexes(toRemove); +} +void ClipboardBrowser::dropIndexes(const QList &indexes) +{ const QPersistentModelIndex current = currentIndex(); - const int first = toRemove.value(0).row(); + const int first = indexes.value(0).row(); // Remove ranges of rows instead of a single rows. - for (auto it1 = std::begin(toRemove); it1 != std::end(toRemove); ) { + for (auto it1 = std::begin(indexes); it1 != std::end(indexes); ) { if ( it1->isValid() ) { const auto firstRow = it1->row(); auto rowCount = 0; - for ( ++it1, ++rowCount; it1 != std::end(toRemove) + for ( ++it1, ++rowCount; it1 != std::end(indexes) && it1->isValid() && it1->row() == firstRow + rowCount; ++it1, ++rowCount ) {} @@ -756,15 +757,28 @@ void ClipboardBrowser::removeIndexes(const QModelIndexList &indexes, QString *er *error = "No valid rows specified"; } - if ( !m_itemSaver->canRemoveItems(indexes, error) ) + if ( !canRemoveItems(indexes, error) ) return; - m_itemSaver->itemsRemovedByUser(indexes); + auto toRemove = toPersistentModelIndexList(indexes); + std::sort( std::begin(toRemove), std::end(toRemove) ); - dropIndexes(indexes); + QPointer self(this); + bool canRemove = true; + emit runOnRemoveItemsHandler(toRemove, &canRemove); + if (!canRemove) { + COPYQ_LOG("Item removal cancelled from script"); + return; + } + if (!self) + return; + + m_itemSaver->itemsRemovedByUser(toRemove); + + dropIndexes(toRemove); } -bool ClipboardBrowser::canRemoveItems(const QModelIndexList &indexes, QString *error) const +bool ClipboardBrowser::canRemoveItems(const QModelIndexList &indexes, QString *error) { Q_ASSERT(m_itemSaver); @@ -1122,6 +1136,8 @@ void ClipboardBrowser::mouseMoveEvent(QMouseEvent *event) QWidget *target = qobject_cast(drag->target()); + QPointer self(this); + // Move items only if target is this app. if (target == this || target == viewport()) { moveIndexes(indexesToRemove, m_dragTargetRow, &m, MoveType::Absolute); @@ -1130,6 +1146,9 @@ void ClipboardBrowser::mouseMoveEvent(QMouseEvent *event) { removeIndexes(selected); } + + if (!self) + return; } // Clear drag indicator. diff --git a/src/gui/clipboardbrowser.h b/src/gui/clipboardbrowser.h index d995a14269..168be466a6 100644 --- a/src/gui/clipboardbrowser.h +++ b/src/gui/clipboardbrowser.h @@ -98,7 +98,7 @@ class ClipboardBrowser final : public QListView void removeIndexes(const QModelIndexList &indexes, QString *error = nullptr); - bool canRemoveItems(const QModelIndexList &indexes, QString *error = nullptr) const; + bool canRemoveItems(const QModelIndexList &indexes, QString *error = nullptr); /** Render preview image with items. */ QPixmap renderItemPreview(const QModelIndexList &indexes, int maxWidth, int maxHeight); @@ -223,6 +223,7 @@ class ClipboardBrowser final : public QListView signals: void itemsAboutToBeRemoved(const QModelIndex &parent, int first, int last); + void runOnRemoveItemsHandler(const QList &indexes, bool *canRemove); /** Show list request. */ void requestShow(const ClipboardBrowser *self); @@ -352,6 +353,7 @@ class ClipboardBrowser final : public QListView /// Removes indexes without notifying or asking plugins. void dropIndexes(const QModelIndexList &indexes); + void dropIndexes(const QList &indexes); void focusEditedIndex(); diff --git a/src/gui/mainwindow.cpp b/src/gui/mainwindow.cpp index c8f92d6aca..c0505f5bf1 100644 --- a/src/gui/mainwindow.cpp +++ b/src/gui/mainwindow.cpp @@ -1279,10 +1279,13 @@ void MainWindow::onBrowserCreated(ClipboardBrowser *browser) connect( browser, &ClipboardBrowser::itemWidgetCreated, this, &MainWindow::onItemWidgetCreated ); - connect( browser, &ClipboardBrowser::itemsAboutToBeRemoved, - browser, [this, browser](const QModelIndex &, int first, int last) { - if (isScriptOverridden(ScriptOverrides::OnItemsRemoved)) - runItemHandlerScript(QStringLiteral("onItemsRemoved()"), browser, first, last); + connect( browser, &ClipboardBrowser::runOnRemoveItemsHandler, + browser, [this, browser](const QList &indexes, bool *canRemove) { + if (isScriptOverridden(ScriptOverrides::OnItemsRemoved)) { + QVariantMap data = createDataMap(mimeCurrentTab, browser->tabName()); + addSelectionData(&data, indexes); + *canRemove = runEventHandlerScript(QStringLiteral("onItemsRemoved()"), data); + } } ); connect( browser->model(), &QAbstractItemModel::rowsInserted, browser, [this, browser](const QModelIndex &, int first, int last) { @@ -1299,7 +1302,7 @@ void MainWindow::onBrowserCreated(ClipboardBrowser *browser) } ); if (isScriptOverridden(ScriptOverrides::OnItemsLoaded)) { - runEventHandlerScript( + runScript( QStringLiteral("onItemsLoaded()"), createDataMap(mimeCurrentTab, browser->tabName())); } @@ -2463,10 +2466,10 @@ Action *MainWindow::runScript(const QString &script, const QVariantMap &data) return act; } -void MainWindow::runEventHandlerScript(const QString &script, const QVariantMap &data) +bool MainWindow::runEventHandlerScript(const QString &script, const QVariantMap &data) { if (m_maxEventHandlerScripts == 0) - return; + return false; --m_maxEventHandlerScripts; if (m_maxEventHandlerScripts == 0) @@ -2478,6 +2481,7 @@ void MainWindow::runEventHandlerScript(const QString &script, const QVariantMap action->waitForFinished(); setUpdatesEnabled(hasUpdatesEnabled); ++m_maxEventHandlerScripts; + return !action->actionFailed() && action->exitCode() == 0; } void MainWindow::runItemHandlerScript( @@ -2493,7 +2497,7 @@ void MainWindow::runItemHandlerScript( QVariantMap data = createDataMap(mimeCurrentTab, browser->tabName()); addSelectionData(&data, indexes); - runEventHandlerScript(script, data); + runScript(script, data); } int MainWindow::findTabIndex(const QString &name) diff --git a/src/gui/mainwindow.h b/src/gui/mainwindow.h index 6c7217c185..ee4c1b33a6 100644 --- a/src/gui/mainwindow.h +++ b/src/gui/mainwindow.h @@ -38,11 +38,11 @@ struct MainWindowOptions; struct NotificationButton; Q_DECLARE_METATYPE(QPersistentModelIndex) -Q_DECLARE_METATYPE(QList) #if QT_VERSION >= QT_VERSION_CHECK(6,0,0) using NativeEventResult = qintptr; #else +Q_DECLARE_METATYPE(QList) using NativeEventResult = long; #endif @@ -629,7 +629,7 @@ class MainWindow final : public QMainWindow const Theme &theme() const; Action *runScript(const QString &script, const QVariantMap &data = QVariantMap()); - void runEventHandlerScript(const QString &script, const QVariantMap &data = QVariantMap()); + bool runEventHandlerScript(const QString &script, const QVariantMap &data); void runItemHandlerScript( const QString &script, const ClipboardBrowser *browser, int firstRow, int lastRow); diff --git a/src/item/itemsaverwrapper.cpp b/src/item/itemsaverwrapper.cpp index 7752f83348..297f588a17 100644 --- a/src/item/itemsaverwrapper.cpp +++ b/src/item/itemsaverwrapper.cpp @@ -27,7 +27,7 @@ bool ItemSaverWrapper::canMoveItems(const QList &indexList) return m_saver->canMoveItems(indexList); } -void ItemSaverWrapper::itemsRemovedByUser(const QList &indexList) +void ItemSaverWrapper::itemsRemovedByUser(const QList &indexList) { return m_saver->itemsRemovedByUser(indexList); } diff --git a/src/item/itemsaverwrapper.h b/src/item/itemsaverwrapper.h index a6b47f60ef..15133057db 100644 --- a/src/item/itemsaverwrapper.h +++ b/src/item/itemsaverwrapper.h @@ -15,7 +15,7 @@ class ItemSaverWrapper : public ItemSaverInterface bool canMoveItems(const QList &indexList) override; - void itemsRemovedByUser(const QList &indexList) override; + void itemsRemovedByUser(const QList &indexList) override; QVariantMap copyItem(const QAbstractItemModel &model, const QVariantMap &itemData) override; diff --git a/src/item/itemwidget.cpp b/src/item/itemwidget.cpp index 840e1f200c..2974bb57d2 100644 --- a/src/item/itemwidget.cpp +++ b/src/item/itemwidget.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -197,7 +198,7 @@ bool ItemSaverInterface::canMoveItems(const QList &) return true; } -void ItemSaverInterface::itemsRemovedByUser(const QList &) +void ItemSaverInterface::itemsRemovedByUser(const QList &) { } diff --git a/src/item/itemwidget.h b/src/item/itemwidget.h index 66d3f0be1f..750d339048 100644 --- a/src/item/itemwidget.h +++ b/src/item/itemwidget.h @@ -17,6 +17,7 @@ class TestInterface; class QAbstractItemModel; class QIODevice; class QModelIndex; +class QPersistentModelIndex; class QSettings; class QTextEdit; class QWidget; @@ -204,7 +205,7 @@ class ItemSaverInterface /** * Called when items are being deleted by user. */ - virtual void itemsRemovedByUser(const QList &indexList); + virtual void itemsRemovedByUser(const QList &indexList); /** * Return copy of items data. diff --git a/src/scriptable/scriptable.cpp b/src/scriptable/scriptable.cpp index 11e31b8312..cc5095bade 100644 --- a/src/scriptable/scriptable.cpp +++ b/src/scriptable/scriptable.cpp @@ -345,7 +345,7 @@ bool isGuiApplication() bool isOverridden(const QJSValue &globalObject, const QString &property) { - return globalObject.property(property).property(QStringLiteral("_copyq")).toInt() != 1; + return globalObject.property(property).property(QStringLiteral("_copyq")).toString() != property; } } // namespace @@ -393,7 +393,7 @@ Scriptable::Scriptable( "if (_copyqHasUncaughtException) throw _copyqUncaughtException;" "return v;" "};" - "f._copyq = 1;" + "f._copyq = name;" "return f;" "})" )); diff --git a/src/tests/tests.cpp b/src/tests/tests.cpp index efbd6940d6..520f6096c1 100644 --- a/src/tests/tests.cpp +++ b/src/tests/tests.cpp @@ -4019,6 +4019,42 @@ void Tests::scriptOnItemsRemoved() RUN("tab" << tab1 << "add(3,2,1,0)", ""); RUN("tab" << tab1 << "remove(1,2)", ""); WAIT_ON_OUTPUT("separator" << "," << "read(0,1,2,)", "R1:2,R0:1,"); + + // Cancel item removal + const auto script2 = R"( + setCommands([ + { + isScript: true, + cmd: "global.onItemsRemoved = global.fail", + }, + ]) + )"; + RUN(script2, ""); + const auto tab2 = testTab(2); + RUN("tab" << tab2 << "add(3,2,1,0)", ""); + RUN("tab" << tab2 << "remove(1,2)", ""); + waitFor(1000); + RUN("tab" << tab2 << "separator" << "," << "read(0,1,2,3,4)", "0,1,2,3,"); + + // Avoid crash if the tab itself is removed while removing items + const auto script3 = R"( + setCommands([ + { + isScript: true, + cmd: ` + global.onItemsRemoved = function() { + removeTab(selectedTab()) + } + ` + }, + ]) + )"; + RUN(script3, ""); + const auto tab3 = testTab(3); + RUN("tab" << tab3 << "add(3,2,1,0)", ""); + RUN("tab" << tab3 << "remove(1,2)", ""); + waitFor(1000); + RUN("tab" << tab3 << "separator" << "," << "read(0,1,2,3,4)", ",,,,"); } void Tests::scriptOnItemsAdded() @@ -4029,10 +4065,10 @@ void Tests::scriptOnItemsAdded() isScript: true, cmd: ` global.onItemsAdded = function() { - if (selectedTab() == tab()[0]) abort(); - items = ItemSelection().current().items(); - tab(tab()[0]); - add("A:" + str(items[0][mimeText])); + sel = ItemSelection().current(); + items = sel.items(); + items[0][mimeText] = "A:" + str(items[0][mimeText]) + sel.setItems(items); } ` }, @@ -4041,7 +4077,7 @@ void Tests::scriptOnItemsAdded() RUN(script, ""); const auto tab1 = testTab(1); RUN("tab" << tab1 << "add(1,0)", ""); - WAIT_ON_OUTPUT("separator" << "," << "read(0,1,2)", "A:0,A:1,"); + WAIT_ON_OUTPUT("tab" << tab1 << "separator" << "," << "read(0,1,2)", "A:0,A:1,"); } void Tests::scriptOnItemsChanged() @@ -4103,16 +4139,20 @@ void Tests::scriptEventMaxRecursion() setCommands([ { isScript: true, - cmd: 'global.onItemsAdded = function() { add("A") }' + cmd: `global.onItemsRemoved = function() { + const toRemove = str(selectedItemData(0)[mimeText]); + const newItem = (toRemove == "X") ? "A" : ("WRONG:" + toRemove); + add(newItem); + remove(size()-1); + }` }, ]) )"; RUN(script, ""); - RUN("add(1,0)", ""); - WAIT_ON_OUTPUT("length", "22\n"); + RUN("add('X'); remove(0)", ""); + WAIT_ON_OUTPUT("separator" << "," << "read(0,1,2,3,4,5,6,7,8,9,10)", "A,A,A,A,A,A,A,A,A,A,"); waitFor(200); - RUN("separator" << "," << "read(0,1,2)", "A,A,A"); - RUN("length", "22\n"); + RUN("separator" << "," << "read(0,1,2,3,4,5,6,7,8,9,10)", "A,A,A,A,A,A,A,A,A,A,"); } void Tests::scriptSlowCollectOverrides()