Review Request 113205: Make KJob::result public for the new signal/slot syntax.

[David Faure]

On Oct. 11, 2013, 9:51 p.m., Mark Gaiser wrote:
> > We are here making a 'hole' for people to do  'bad things' that wasn't possible in the past. I'm not sure we want that.
> 
> Mark Gaiser wrote:
>     Interesting.
>     So that mean we simply can't use the new signal/slot syntax because of it? That would seem rather strange to me..
>     
>     If you do a stat call, or listEntry or <any of the others>...
>     Then you are supposed to connect to the result slot. For listEntry you are supposed to connect to the finished signal. Both of those are defined as:
>     signals:
>     private:
>     
>     AKA. Private signals.
>     I really don't see how you can work around this besides perhaps QSignalMapper, but that would be very odd as well. I'm really curious to see how that "bit of magic" is supposed to work. Do you have some links for me there?
> 
> Sune Vuorela wrote:
>     I'm not saying we can't use the new syntax because of it. I'm saying it needs a bit more work, and before a 'stable' version is needed.
>     
>     There is a solution out there. It's applied to QAIM and others.
> 
> Mark Gaiser wrote:
>     Oke, i followed the hints and now know what you mean. The question now becomes: do we want this? Preventing this hole and supporting the new signal/slot syntax is possible.
>     
>     This does change the header and all places where this signal is currently being called. The header would look like (with the signal moved to the public part):
>     void result(KJob *job
>     #if !defined(qdoc)
>     , QPrivateSignal
>     #endif
>     );
>     
>     To source files where this signal is emitted would change to:
>     result(job, QPrivateSignal());
>     
>     Do we want this?

Yes. I quote: "This is a private signal, it can't be emitted directly by subclasses of KJob, use emitResult() instead."

So only kjob.cpp will have to be updated.

The #if should be
#if !defined(DOXYGEN_SHOULD_SKIP_THIS)
though.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113205/#review41570
-----------------------------------------------------------


On Oct. 12, 2013, 2:08 p.m., Mark Gaiser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113205/
> -----------------------------------------------------------
> 
> (Updated Oct. 12, 2013, 2:08 p.m.)
> 
> 
> Review request for KDE Frameworks, kdelibs, David Faure, and Kevin Ottens.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> The new signal/slot connection:
> connect(job, &KJob::result,...
> 
> does't like result to be private and throws an compile error:
> error: 'void KJob::result(KJob*)' is private
> 
> Making it public resolves the issue and makes this slot usable in the new syntax. In my case i wanted to use the new syntax and directly use a lambda as slot. Which isn't possible on this signal if it isn't public.
> 
> 
> Diffs
> -----
> 
>   tier1/kcoreaddons/src/lib/jobs/kjob.h d663530 
> 
> Diff: http://git.reviewboard.kde.org/r/113205/diff/
> 
> 
> Testing
> -------
> 
> Works just fine.
> 
> 
> Thanks,
> 
> Mark Gaiser
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20131012/4fcaa743/attachment.html>