Git commit cef296113612ae72e561a1300d0622a46fcf1906 by Amish Naidu. Committed on 28/09/2018 at 14:58. Pushed by anaidu into branch '5.3'. Fix removal of external scripts Summary: Previously, deleting a script which is in the middle of the config would in= validate indices of the scripts that come after it in the config, this would mean deleting anything later deletes the wrong script. The config labels are now stored in the model itself and is no longer based= on just the index, but on the name of the script. Thus, deleting/updating one script does not = interfere with the other scripts. BUG: 385298 Test Plan: * Before applying the patch: - Delete "Google Selection" - Restart - Delete "Quick Compile" - Restart and check list of external scripts * Apply the patch and repeat. Reviewers: #kdevelop, flherne Reviewed By: #kdevelop, flherne Subscribers: flherne, kdevelop-devel Tags: #kdevelop Differential Revision: https://phabricator.kde.org/D15743 M +11 -0 plugins/externalscript/externalscriptitem.cpp M +11 -0 plugins/externalscript/externalscriptitem.h M +34 -9 plugins/externalscript/externalscriptplugin.cpp M +12 -2 plugins/externalscript/externalscriptplugin.h https://commits.kde.org/kdevelop/cef296113612ae72e561a1300d0622a46fcf1906 diff --git a/plugins/externalscript/externalscriptitem.cpp b/plugins/extern= alscript/externalscriptitem.cpp index aa1ea3a691..3224e50441 100644 --- a/plugins/externalscript/externalscriptitem.cpp +++ b/plugins/externalscript/externalscriptitem.cpp @@ -34,6 +34,17 @@ ExternalScriptItem::ExternalScriptItem() { } = +QString ExternalScriptItem::key() const +{ + return m_key; +} + +void ExternalScriptItem::setKey(const QString& key) +{ + m_key =3D key; +} + + QString ExternalScriptItem::command() const { return m_command; diff --git a/plugins/externalscript/externalscriptitem.h b/plugins/external= script/externalscriptitem.h index 192d7c679a..edfafe7e0a 100644 --- a/plugins/externalscript/externalscriptitem.h +++ b/plugins/externalscript/externalscriptitem.h @@ -33,6 +33,16 @@ class ExternalScriptItem : public QStandardItem public: ExternalScriptItem(); = + /** + * The key is supposed to be unique inside the model + * @return The key of this script item + */ + QString key() const; + /** + * Sets the label + */ + void setKey( const QString& key ); + /** * @return The command to execute. */ @@ -198,6 +208,7 @@ public: */ void save() const; private: + QString m_key; QString m_command; QString m_workingDirectory; SaveMode m_saveMode =3D SaveNone; diff --git a/plugins/externalscript/externalscriptplugin.cpp b/plugins/exte= rnalscript/externalscriptplugin.cpp index d0389586e1..89b70eb7f4 100644 --- a/plugins/externalscript/externalscriptplugin.cpp +++ b/plugins/externalscript/externalscriptplugin.cpp @@ -108,6 +108,7 @@ ExternalScriptPlugin::ExternalScriptPlugin( QObject* pa= rent, const QVariantList& KConfigGroup script =3D config.group( group ); if ( script.hasKey( "name" ) && script.hasKey( "command" ) ) { ExternalScriptItem* item =3D new ExternalScriptItem; + item->setKey( script.name() ); item->setText( script.readEntry( "name" ) ); item->setCommand( script.readEntry( "command" )); item->setInputMode( static_cast( scri= pt.readEntry( "inputMode", 0u ) ) ); @@ -124,8 +125,8 @@ ExternalScriptPlugin::ExternalScriptPlugin( QObject* pa= rent, const QVariantList& = core()->uiController()->addToolView( i18n( "External Scripts" ), m_facto= ry ); = - connect( m_model, &QStandardItemModel::rowsRemoved, - this, &ExternalScriptPlugin::rowsRemoved ); + connect( m_model, &QStandardItemModel::rowsAboutToBeRemoved, + this, &ExternalScriptPlugin::rowsAboutToBeRemoved ); connect( m_model, &QStandardItemModel::rowsInserted, this, &ExternalScriptPlugin::rowsInserted ); = @@ -335,16 +336,18 @@ void ExternalScriptPlugin::executeScriptFromContextMe= nu() const = void ExternalScriptPlugin::rowsInserted( const QModelIndex& /*parent*/, in= t start, int end ) { - for ( int i =3D start; i <=3D end; ++i ) { - saveItemForRow( i ); + setupKeys( start, end ); + for ( int row =3D start; row <=3D end; ++row ) { + saveItemForRow( row ); } } = -void ExternalScriptPlugin::rowsRemoved( const QModelIndex& /*parent*/, int= start, int end ) +void ExternalScriptPlugin::rowsAboutToBeRemoved( const QModelIndex& /*pare= nt*/, int start, int end ) { KConfigGroup config =3D getConfig(); - for ( int i =3D start; i <=3D end; ++i ) { - KConfigGroup child =3D config.group( QStringLiteral("script %1").arg(i= ) ); + for ( int row =3D start; row <=3D end; ++row ) { + const ExternalScriptItem* const item =3D static_cast( m_model->item( row ) ); + KConfigGroup child =3D config.group( item->key() ); qCDebug(PLUGIN_EXTERNALSCRIPT) << "removing config group:" << child.na= me(); child.deleteGroup(); } @@ -355,7 +358,10 @@ void ExternalScriptPlugin::saveItem( const ExternalScr= iptItem* item ) { const QModelIndex index =3D m_model->indexFromItem( item ); Q_ASSERT( index.isValid() ); - saveItemForRow( index.row() ); + + getConfig().group( item->key() ).deleteGroup(); // delete the previous g= roup + setupKeys( index.row(), index.row() ); + saveItemForRow( index.row() ); // save the new group } = void ExternalScriptPlugin::saveItemForRow( int row ) @@ -367,7 +373,7 @@ void ExternalScriptPlugin::saveItemForRow( int row ) Q_ASSERT( item ); = qCDebug(PLUGIN_EXTERNALSCRIPT) << "save extern script:" << item << idx; - KConfigGroup config =3D getConfig().group( QStringLiteral("script %1").a= rg( row ) ); + KConfigGroup config =3D getConfig().group( item->key() ); config.writeEntry( "name", item->text() ); config.writeEntry( "command", item->command() ); config.writeEntry( "inputMode", (uint) item->inputMode() ); @@ -380,6 +386,25 @@ void ExternalScriptPlugin::saveItemForRow( int row ) config.sync(); } = +void ExternalScriptPlugin::setupKeys(int start, int end) +{ + QStringList keys =3D getConfig().groupList(); + + for ( int row =3D start; row <=3D end; ++row ) { + ExternalScriptItem* const item =3D static_cast( m= _model->item( row ) ); + + int nextSuffix =3D 2; + QString keyCandidate =3D item->text(); + for (; keys.contains( keyCandidate ); ++nextSuffix) { + keyCandidate =3D item->text() + QString::number( nextSuffix ); + } + + qCDebug(PLUGIN_EXTERNALSCRIPT) << "set key" << keyCandidate << "for" <= < item << item->command(); + item->setKey(keyCandidate); + keys.push_back(keyCandidate); + } +} + #include "externalscriptplugin.moc" = // kate: indent-mode cstyle; space-indent on; indent-width 2; replace-tabs= on; diff --git a/plugins/externalscript/externalscriptplugin.h b/plugins/extern= alscript/externalscriptplugin.h index 9276395364..e5ad8c4818 100644 --- a/plugins/externalscript/externalscriptplugin.h +++ b/plugins/externalscript/externalscriptplugin.h @@ -68,6 +68,9 @@ public: */ KConfigGroup getConfig() const; = + /** + * Saves the @p script to the config and updates the key + */ void saveItem(const ExternalScriptItem* item); = public Q_SLOTS: @@ -82,9 +85,9 @@ public Q_SLOTS: * Executes the command synchronously and returns the output text (Used = by the shell-integration) * */ Q_SCRIPTABLE QString executeCommandSync(const QString& command, const QS= tring& workingDirectory) const; - = + private Q_SLOTS: - void rowsRemoved( const QModelIndex& parent, int start, int end ); + void rowsAboutToBeRemoved( const QModelIndex& parent, int start, int end= ); void rowsInserted( const QModelIndex& parent, int start, int end ); void executeScriptFromContextMenu() const; = @@ -92,6 +95,13 @@ private: /// @param row row in the model for the item to save void saveItemForRow( int row ); = + /** + * Sets up unique keys for items in the range [start, end] + * @param start start of the range + * @param end end of the range + */ + void setupKeys( int start, int end ); + QStandardItemModel* m_model; QList m_urls; static ExternalScriptPlugin* m_self;