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

René J.V. Bertin rjvbertin at gmail.com
Mon Jan 18 20:46:59 UTC 2016



> On Jan. 18, 2016, 5:50 p.m., Kevin Funk wrote:
> > app/main.cpp, line 267
> > <https://git.reviewboard.kde.org/r/126761/diff/2/?file=434222#file434222line267>
> >
> >     Style: `if (!foo) {`
> >     
> >     Early-return:
> >     
> >     ```
> >     if (isEmpty)
> >        return;
> >     
> >     ...
> >     ```

I've tried to fit in with the surrounding style, which isn't exactly consistent ...


> On Jan. 18, 2016, 5:50 p.m., Kevin Funk wrote:
> > app/main.cpp, line 127
> > <https://git.reviewboard.kde.org/r/126761/diff/2/?file=434222#file434222line127>
> >
> >     `struct` not needed

In fact, yes it is. At least with clang:

```
app/main.cpp:127:13: error: 
      use of undeclared identifier 'UrlInfo'
    QVector<UrlInfo> fileOpenRequestList;
            ^
```


> On Jan. 18, 2016, 5:50 p.m., Kevin Funk wrote:
> > app/main.cpp, line 623
> > <https://git.reviewboard.kde.org/r/126761/diff/2/?file=434222#file434222line623>
> >
> >     Wrap in `#ifdef Q_OS_OSX`, too?

That's what I proposed when working with Christoph on the feature implementation for Kate. His POV: no need to introduce an additional #ifdef.

If the consensus is different here, I'd propose putting the ifdefs in the functions, and not burden main() with more conditional code.


> On Jan. 18, 2016, 5:50 p.m., Kevin Funk wrote:
> > app/main.cpp, line 633
> > <https://git.reviewboard.kde.org/r/126761/diff/2/?file=434222#file434222line633>
> >
> >     Pass the PID here? Then you don't need all the changes around `session`?

I can have another look, but I think there are at least 1 or 2 locations where I do need the name of the session the user picked


- René J.V.


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


On Jan. 18, 2016, 4: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, 4: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/ae8cde2b/attachment-0001.html>


More information about the KDevelop-devel mailing list