Review Request 126761: [OS X] handle LaunchServices requests to open files (i.e. from the Finder)

[Kevin Funk]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126761/#review91272
-----------------------------------------------------------



General remarks: Please try to simplify this code as much as possible, and restrict platform-specific code as much as possible.


app/main.cpp (line 93)
<https://git.reviewboard.kde.org/r/126761/#comment62388>

    Nitpick: `{` on next line
    
    Code: Deinline, too (consistency with other function)



app/main.cpp (line 112)
<https://git.reviewboard.kde.org/r/126761/#comment62398>

    Add `override`



app/main.cpp (line 124)
<https://git.reviewboard.kde.org/r/126761/#comment62400>

    Please double-check if this member var is needed, see my comments below.



app/main.cpp (line 126)
<https://git.reviewboard.kde.org/r/126761/#comment62397>

    Init: `= false`



app/main.cpp (line 127)
<https://git.reviewboard.kde.org/r/126761/#comment62396>

    `struct` not needed



app/main.cpp (line 208)
<https://git.reviewboard.kde.org/r/126761/#comment62395>

    Bad API, please revert if possible. See my comment re. `session` below.



app/main.cpp (line 266)
<https://git.reviewboard.kde.org/r/126761/#comment62393>

    Style: `if (!foo) {`
    
    Early-return:
    
    ```
    if (isEmpty)
       return;
    
    ...
    ```



app/main.cpp (line 268)
<https://git.reviewboard.kde.org/r/126761/#comment62392>

    Style: `if (foo) {`



app/main.cpp (line 282)
<https://git.reviewboard.kde.org/r/126761/#comment62394>

    Proper naming: `fileOpenEvent`



app/main.cpp (line 285)
<https://git.reviewboard.kde.org/r/126761/#comment62391>

    Style: `if (!foo) {`



app/main.cpp (line 292)
<https://git.reviewboard.kde.org/r/126761/#comment62390>

    Remove this comment, stating the obvious.



app/main.cpp (line 621)
<https://git.reviewboard.kde.org/r/126761/#comment62399>

    Wrap in `#ifdef Q_OS_OSX`, too?



app/main.cpp (line 631)
<https://git.reviewboard.kde.org/r/126761/#comment62389>

    Pass the PID here? Then you don't need all the changes around `session`?


- Kevin Funk


On Jan. 18, 2016, 3:46 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126761/
> -----------------------------------------------------------
> 
> (Updated Jan. 18, 2016, 3:46 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevelop
> 
> 
> Description
> -------
> 
> This patch introduces support for file open requests from for instance the Finder, on OS X. It also makes it possible to launch KDevelop from the Finder (or the Dock) with the Command key held, in which case the session picker dialog will be presented if multiple sessions are open.
> 
> 
> Diffs
> -----
> 
>   app/Info.plist.in be256f9 
>   app/main.cpp b5f7bb9 
> 
> Diff: https://git.reviewboard.kde.org/r/126761/diff/
> 
> 
> Testing
> -------
> 
> On OS X 10.9.5 with Qt 5.5.1 and KF5 Frameworks 5.17.0 installed in /opt/local
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20160118/1a3c5a39/attachment-0001.html>