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

List:       kwrite-devel
Subject:    Re: SmartRanges... ...and how to handle them correctly :) (please
From:       Dominik Haumann <dhdev () gmx ! de>
Date:       2008-04-27 19:54:01
Message-ID: 200804272154.03839.dhdev () gmx ! de
[Download RAW message or body]

More thoughts on this:

reloading always delets smart ranges from plugins, this is potentially 
dangerous. There are two ways: either fix it in the plugin, or fix it in 
the part. Attached is a patch that fixes it in the part, the idea is to 
handle reloading as "close & open", i.e. all plugins are disabled for the 
document and then enabled again. This fixes the crash in the word 
completion plugin.

Now should we apply the patch? ...or should we rather really fix it in the 
plugins? In theory, a plugin A can delete a smart range of plugin B, so it 
still has to listen on the primaryNotifier(), correct?

Opinions welcome.

Dominik

On Wednesday 16 April 2008, Dominik Haumann wrote:
> Ok, an update on this:
> - if you use smart ranges inside kate part, mark them with setInternal().
>   Kate Part won't delete them on doc reload then.
> - if you use smart ranges outside of kate part (e.g. KTE plugins), always
>   use a primaryNotifier() to detect when a range got deleted.
>
> That said, the word completion plugin crashes because of this, too, for
> example... (ctrl+8/9, then reload, then again ctrl+8/9) ...needs to be
> fixed.
>
> Dominik
>
> On Wednesday 09 April 2008, Dominik Haumann wrote:
> > Btw, as far as I can see, this means that:
> >   whenever you use smart-ranges and use or delete them yourself later
> > you have to connect to the rangeDeleted() signal.
> >
> > Otherwise you'll have a dangling pointer.
> >
> > On Wednesday 09 April 2008, Dominik Haumann wrote:
> > > Hi,
> > >
> > > the same crash also exists in search:
> > > 1. ctrl+f, type text
> > > 2. now the match is highlighted
> > > 3. reload the doc (F5)
> > > 4. close the searchbar
> > > 5. crash
> > >
> > > The fix is obviously to use the primaryNotifier() to get notified
> > > when the ranges are deleted.
> > >
> > > Dominik
> > > [...]


["katepart-smartrange-workaround.hdiff" (text/x-diff)]

Index: utils/katepartpluginmanager.cpp
===================================================================
--- utils/katepartpluginmanager.cpp	(Revision 801743)
+++ utils/katepartpluginmanager.cpp	(Arbeitskopie)
@@ -69,7 +69,7 @@
 {
   // NOTE: adapt the interval each minor KDE version
   KService::List traderList = KServiceTypeTrader::self()->
-      query("KTextEditor/Plugin", "([X-KDE-Version] >= 4.0) and ([X-KDE-Version] <= 4.0)");
+      query("KTextEditor/Plugin", "([X-KDE-Version] >= 4.0) and ([X-KDE-Version] <= 4.1)");
 
   foreach(const KService::Ptr &ptr, traderList)
   {
@@ -250,4 +250,28 @@
   }
 }
 
+void KatePartPluginManager::removeAllPluginsForDocument(KTextEditor::Document *doc)
+{
+  foreach (KTextEditor::View *view, doc->views()) {
+    KXMLGUIFactory *viewFactory = view->factory();
+    if (viewFactory) viewFactory->removeClient(view);
+    removeView(view);
+    if (viewFactory) viewFactory->addClient(view);
+  }
+  removeDocument(doc);
+}
+
+void KatePartPluginManager::addAllPluginsForDocument(KTextEditor::Document *doc)
+{
+  addDocument(doc);
+  foreach (KTextEditor::View *view, doc->views())
+  {
+    KXMLGUIFactory *viewFactory = view->factory();
+    if (viewFactory) viewFactory->removeClient(view);
+    addView(view);
+    if (viewFactory) viewFactory->addClient(view);
+  }
+}
+
+
 // kate: space-indent on; indent-width 2; replace-tabs on;
Index: utils/katepartpluginmanager.h
===================================================================
--- utils/katepartpluginmanager.h	(Revision 801743)
+++ utils/katepartpluginmanager.h	(Arbeitskopie)
@@ -74,6 +74,9 @@
     void enablePlugin (KatePartPluginInfo &item);
     void disablePlugin (KatePartPluginInfo &item);
 
+    void removeAllPluginsForDocument(KTextEditor::Document *doc);
+    void addAllPluginsForDocument(KTextEditor::Document *doc);
+
     inline KatePartPluginList & pluginList ()
     {
       return m_pluginList;
Index: document/katedocument.cpp
===================================================================
--- document/katedocument.cpp	(Revision 801743)
+++ document/katedocument.cpp	(Arbeitskopie)
       cursorPositions.append( v->cursorPosition() );
 
     m_reloading = true;
+    KatePartPluginManager::self()->removeAllPluginsForDocument(this);
     KateDocument::openUrl( url() );
+    KatePartPluginManager::self()->addAllPluginsForDocument(this);
     m_reloading = false;
 
     // restore cursor positions for all views


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