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

List:       kwrite-devel
Subject:    Re: Re: [kdevplatform/frameworks] /: Port to KTextEditor::TextHintProvider
From:       Aleix Pol <aleixpol () kde ! org>
Date:       2014-01-28 14:03:10
Message-ID: CACcA1Rr9uv5BJ-kz=ypWEUqz0NoF7xYszpBWK+dJfn012zqJ4Q () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


On Tue, Jan 28, 2014 at 10:31 AM, Dominik Haumann <dhaumann@kde.org> wrote:

> On Tuesday, January 28, 2014 10:11:18 Dominik Haumann wrote:
> > On Tuesday, January 28, 2014 03:16:17 Aleix Pol wrote:
> > > Git commit 249292cc0b7cba90c8344283b157ae82dc7684d0 by Aleix Pol.
> > > Committed on 28/01/2014 at 03:14.
> > > Pushed by apol into branch 'frameworks'.
> > >
> > > Port to KTextEditor::TextHintProvider
> > >
> > > M  +1    -2    debugger/CMakeLists.txt
> > > M  +15   -19   debugger/variable/variablecollection.cpp
> > > M  +13   -2    debugger/variable/variablecollection.h
> > > M  +14   -10   plugins/contextbrowser/contextbrowser.cpp
> > > M  +13   -2    plugins/contextbrowser/contextbrowser.h
> > > M  +12   -9    plugins/problemreporter/problemhighlighter.cpp
> > > M  +14   -1    plugins/problemreporter/problemhighlighter.h
> > >
> > > +    iface->registerTextHintProvider(new VariableProvider(this));
> >
> > This patch looks much more correct.
> >
> > The only thing that is missing is an _un_registerTextHintProvider(), or
> do
> > your TextHintProviders always live longer than the KTextEditor::Views?
> >
> > If a text hint provider is deleted, then KTextEditor::View has a dangling
> > pointer to your deleted text hint provider.
>
> Alex, I can change the interface as follows:
>
> class KTEXTEDITOR_EXPORT TextHintProvider : public QObject
> {
> ...
> };
>
> This way, Kate can internally use a QPointer to check when the provider
> gets
> invalid, so we can at least avoid the dangling pointer and a potential
> crash
> in case the unregisterTextHintProvider is missing.
>
> Is that wanted?
>
> Greetings,
> Dominik
>

Ah, I expected it to be deleted with the view.
Yes, it would make sense to parent them at least.

Aleix

[Attachment #5 (text/html)]

<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Jan 28, 2014 \
at 10:31 AM, Dominik Haumann <span dir="ltr">&lt;<a href="mailto:dhaumann@kde.org" \
target="_blank">dhaumann@kde.org</a>&gt;</span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tuesday, January 28, \
2014 10:11:18 Dominik Haumann wrote:<br> &gt; On Tuesday, January 28, 2014 03:16:17 \
Aleix Pol wrote:<br> &gt; &gt; Git commit 249292cc0b7cba90c8344283b157ae82dc7684d0 by \
Aleix Pol.<br> &gt; &gt; Committed on 28/01/2014 at 03:14.<br>
&gt; &gt; Pushed by apol into branch &#39;frameworks&#39;.<br>
&gt; &gt;<br>
&gt; &gt; Port to KTextEditor::TextHintProvider<br>
&gt; &gt;<br>
&gt; &gt; M   +1      -2      debugger/CMakeLists.txt<br>
&gt; &gt; M   +15    -19    debugger/variable/variablecollection.cpp<br>
&gt; &gt; M   +13    -2      debugger/variable/variablecollection.h<br>
&gt; &gt; M   +14    -10    plugins/contextbrowser/contextbrowser.cpp<br>
&gt; &gt; M   +13    -2      plugins/contextbrowser/contextbrowser.h<br>
&gt; &gt; M   +12    -9      plugins/problemreporter/problemhighlighter.cpp<br>
&gt; &gt; M   +14    -1      plugins/problemreporter/problemhighlighter.h<br>
&gt; &gt;<br>
&gt; &gt; +      iface-&gt;registerTextHintProvider(new VariableProvider(this));<br>
&gt;<br>
&gt; This patch looks much more correct.<br>
&gt;<br>
&gt; The only thing that is missing is an _un_registerTextHintProvider(), or do<br>
&gt; your TextHintProviders always live longer than the KTextEditor::Views?<br>
&gt;<br>
&gt; If a text hint provider is deleted, then KTextEditor::View has a dangling<br>
&gt; pointer to your deleted text hint provider.<br>
<br>
</div></div>Alex, I can change the interface as follows:<br>
<br>
class KTEXTEDITOR_EXPORT TextHintProvider : public QObject<br>
{<br>
...<br>
};<br>
<br>
This way, Kate can internally use a QPointer to check when the provider gets<br>
invalid, so we can at least avoid the dangling pointer and a potential crash<br>
in case the unregisterTextHintProvider is missing.<br>
<br>
Is that wanted?<br>
<br>
Greetings,<br>
Dominik<br>
</blockquote></div><br></div><div class="gmail_extra">Ah, I expected it to be deleted \
with the view.</div><div class="gmail_extra">Yes, it would make sense to parent them \
at least.</div><div class="gmail_extra"><br></div>

<div class="gmail_extra">Aleix</div></div>



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