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

[René J.V. Bertin]

> On Jan. 17, 2016, 1:08 p.m., Milian Wolff wrote:
> > app/main.cpp, line 93
> > <https://git.reviewboard.kde.org/r/126761/diff/1/?file=431004#file431004line93>
> >
> >     rename to `startHandleFileOpenEvents`.
> >     
> >     why can't you do this from the beginning btw? i.e. plain `event` overload instead of an event filter on yourself?

My initial approach installed the event filter in the KDevelopApplication ctor. The problem with that is that `FileOpen` events can be caught too early in that case. One of the effects of that was that "opening" a file in KDevelop through the Finder would not hand the file off to the selected session, when multiple sessions were open and I picked a specific session. Or it would, but another session would be activated. It took me a while to figure out why that happened, and also to find the most appropriate place to install the event filter.

Is that a sufficient and acceptable answer?


> On Jan. 17, 2016, 1:08 p.m., Milian Wolff wrote:
> > app/main.cpp, line 413
> > <https://git.reviewboard.kde.org/r/126761/diff/1/?file=431004#file431004line413>
> >
> >     `==` instead of `&`? I.e. if shift + controll is pressed, this should not trigger, or?

Indeed, it should only trigger when the single modifier is pressed. There are already a number of modifier combinations that change launching behaviour at least one of which includes the Control key.

BTW, that's actually the Command ("Apple") key. Qt decided in their wisdom that they have no use for a dedicated shortcut/accelerator modifier token (which would map to `ControlModifier` on MS Windows and Linux, to `MetaModifier` or `CommandModifier` on OS X) and that it was just as easy to swap Control and Meta by default on OS X. Swapping that happens on multiple locations, to make things easy...


> On Jan. 17, 2016, 1:08 p.m., Milian Wolff wrote:
> > app/main.cpp, line 503
> > <https://git.reviewboard.kde.org/r/126761/diff/1/?file=431004#file431004line503>
> >
> >     remove, put it into the commit message

Note to self: don't forget to add this to the commit message:

```
    // Replaced the local QString session var. with app.session so the FileOpen event handler
    // can override the selected session
```

(as if that's any guarantee =) )


> On Jan. 17, 2016, 1:08 p.m., Milian Wolff wrote:
> > app/main.cpp, line 567
> > <https://git.reviewboard.kde.org/r/126761/diff/1/?file=431004#file431004line567>
> >
> >     bad replace

Was that about the comment (`// app.session doesn't exist, we can create it`)?


- René J.V.


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


On Jan. 15, 2016, 4:50 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. 15, 2016, 4:50 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/663298a7/attachment.html>