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

List:       kwrite-devel
Subject:    Re: X-KDE-PluginInfo-Depends not working for KTextEditor Plugins?
From:       Milian Wolff <mail () milianw ! de>
Date:       2008-12-23 13:34:48
Message-ID: 200812231434.52288.mail () milianw ! de
[Download RAW message or body]

[Attachment #2 (multipart/signed)]

[Attachment #4 (multipart/mixed)]


Am Dienstag 23 Dezember 2008 schrieb Christoph Cullmann:
> On Monday 22 December 2008 20:33:22 Milian Wolff wrote:
> > If the patch is OK I can commit it to trunk. But can I also backport it
> > to the KDE4.2 branch? I mean it's a bugfix in my eyes... If I should
> > backport it, how would I do that? I've once read about a script which
> > does just that. Cannot find it documented on techbase though.
>
> Looks ok for me for both.

Great!

> Circular dependencies could be a problem, thought, perhaps therefor, the
> load/unload state should be set before the recursion happens, that at least
> no endless loop is done.

Do you mean item.load ? I fear that this is not so easy, take a look at 
loadAllPlugins for example, it has the following conditional:

if ( it->load )
{
	loadPlugin(*it);
	enablePlugin(*it);
}

So it->load is true before the plugin is even loaded! I.e. I can't simply do a 
item.load = true before my loop and check if ( !it->load ) loadPlugin( *it ); 
inside it...

I think the "load" boolean just tells whether the plugin should be started 
automatically, i.e. look at loadConfig().

And I won't be able to check it->plugin since I cannot load it beforehand.

So: I'll stick to the "don't detect circular references" mantra. Developers 
should spot them themselves ;-) 

> Beside, could you avoid the == 0 compares? if (!something) just looks nicer
> imho for pointer tests. (But I guess me introduced some of them myself :))

Didn't like them either, that's why I added that one comment asking about the 
right style. Updated patch is attached.

If we skip the circular references check I could commit, right? And I'll need 
hints on how to backport.

-- 
Milian Wolff
mail@milianw.de
http://milianw.de

["katepartpluginmanager_dependencies.patch" (text/x-patch)]

Index: katepartpluginmanager.cpp
===================================================================
--- katepartpluginmanager.cpp	(Revision 900386)
+++ katepartpluginmanager.cpp	(Arbeitskopie)
@@ -30,6 +30,7 @@
 #include <kconfig.h>
 #include <kconfiggroup.h>
 #include <kxmlguifactory.h>
+#include <kplugininfo.h>
 
 #include <kservicetypetrader.h>
 #include <kdebug.h>
@@ -184,6 +185,22 @@ void KatePartPluginManager::loadPlugin (
 {
   if (item.plugin) return;
 
+  // make sure all dependencies are loaded beforehand
+  QStringList openDependencies = KPluginInfo( item.service ).dependencies();
+  if ( !openDependencies.empty() )
+  {
+    for (KatePartPluginList::iterator it = m_pluginList.begin();
+      it != m_pluginList.end(); ++it)
+    {
+      if ( openDependencies.contains( it->saveName() ) )
+      {
+        loadPlugin( *it );
+        openDependencies.removeAll( it->saveName() );
+      }
+    }
+    Q_ASSERT( openDependencies.empty() );
+  }
+
   item.plugin = KTextEditor::createPlugin (item.service, this);
   Q_ASSERT(item.plugin);
   item.load = (item.plugin != 0);
@@ -191,6 +208,20 @@ void KatePartPluginManager::loadPlugin (
 
 void KatePartPluginManager::unloadPlugin (KatePartPluginInfo &item)
 {
+  if ( !item.plugin ) return;
+
+  // make sure dependent plugins are unloaded beforehand
+  for (KatePartPluginList::iterator it = m_pluginList.begin();
+    it != m_pluginList.end(); ++it)
+  {
+    if ( !it->plugin ) continue;
+
+    if ( KPluginInfo( it->service ).dependencies().contains( item.saveName() ) )
+    {
+      unloadPlugin( *it );
+    }
+  }
+
   delete item.plugin;
   item.plugin = 0L;
   item.load = false;

["signature.asc" (application/pgp-signature)]

_______________________________________________
KWrite-Devel mailing list
KWrite-Devel@kde.org
https://mail.kde.org/mailman/listinfo/kwrite-devel


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

Configure | About | News | Add a list | Sponsored by KoreLogic