[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [kdevelop/5.3] plugins/externalscript: Fix removal of external scripts
From:       Amish Naidu <null () kde ! org>
Date:       2018-09-28 14:58:46
Message-ID: E1g5uEA-0007Tt-1c () code ! kde ! org
[Download RAW message or body]

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 invalidate \
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/externalscript/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 = key;
+}
+
+
 QString ExternalScriptItem::command() const
 {
   return m_command;
diff --git a/plugins/externalscript/externalscriptitem.h \
b/plugins/externalscript/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 = SaveNone;
diff --git a/plugins/externalscript/externalscriptplugin.cpp \
b/plugins/externalscript/externalscriptplugin.cpp index d0389586e1..89b70eb7f4 100644
--- a/plugins/externalscript/externalscriptplugin.cpp
+++ b/plugins/externalscript/externalscriptplugin.cpp
@@ -108,6 +108,7 @@ ExternalScriptPlugin::ExternalScriptPlugin( QObject* parent, \
const QVariantList&  KConfigGroup script = config.group( group );
     if ( script.hasKey( "name" ) && script.hasKey( "command" ) ) {
       ExternalScriptItem* item = new ExternalScriptItem;
+      item->setKey( script.name() );
       item->setText( script.readEntry( "name" ) );
       item->setCommand( script.readEntry( "command" ));
       item->setInputMode( static_cast<ExternalScriptItem::InputMode>( \
script.readEntry( "inputMode", 0u ) ) ); @@ -124,8 +125,8 @@ \
ExternalScriptPlugin::ExternalScriptPlugin( QObject* parent, const QVariantList&  
   core()->uiController()->addToolView( i18n( "External Scripts" ), m_factory );
 
-  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::executeScriptFromContextMenu() const
 
 void ExternalScriptPlugin::rowsInserted( const QModelIndex& /*parent*/, int start, \
int end )  {
-  for ( int i = start; i <= end; ++i ) {
-    saveItemForRow( i );
+  setupKeys( start, end );
+  for ( int row = start; row <= end; ++row ) {
+    saveItemForRow( row );
   }
 }
 
-void ExternalScriptPlugin::rowsRemoved( const QModelIndex& /*parent*/, int start, \
int end ) +void ExternalScriptPlugin::rowsAboutToBeRemoved( const QModelIndex& \
/*parent*/, int start, int end )  {
   KConfigGroup config = getConfig();
-  for ( int i = start; i <= end; ++i ) {
-    KConfigGroup child = config.group( QStringLiteral("script %1").arg(i) );
+  for ( int row = start; row <= end; ++row ) {
+    const ExternalScriptItem* const item = static_cast<ExternalScriptItem*>( \
m_model->item( row ) ); +    KConfigGroup child = config.group( item->key() );
     qCDebug(PLUGIN_EXTERNALSCRIPT) << "removing config group:" << child.name();
     child.deleteGroup();
   }
@@ -355,7 +358,10 @@ void ExternalScriptPlugin::saveItem( const ExternalScriptItem* \
item )  {
   const QModelIndex index = m_model->indexFromItem( item );
   Q_ASSERT( index.isValid() );
-  saveItemForRow( index.row() );
+
+  getConfig().group( item->key() ).deleteGroup(); // delete the previous group
+  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 = getConfig().group( QStringLiteral("script %1").arg( row ) );
+  KConfigGroup config = 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 = getConfig().groupList();
+
+  for ( int row = start; row <= end; ++row ) {
+    ExternalScriptItem* const item = static_cast<ExternalScriptItem*>( \
m_model->item( row ) ); +
+    int nextSuffix = 2;
+    QString keyCandidate = item->text();
+    for (; keys.contains( keyCandidate ); ++nextSuffix) {
+      keyCandidate = 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/externalscript/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 QString& \
                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<QUrl> m_urls;
   static ExternalScriptPlugin* m_self;


[prev in list] [next in list] [prev in thread] [next in thread]