[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]