Bug 217810 - [KeyBindings] org.eclipse.ui.bindings -> key becomes awkward with carbon and cocoa
Summary: [KeyBindings] org.eclipse.ui.bindings -> key becomes awkward with carbon and ...
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.4   Edit
Hardware: PC All
: P3 normal (vote)
Target Milestone: 3.5 M3   Edit
Assignee: Paul Webster CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 296927
  Show dependency tree
 
Reported: 2008-02-05 06:27 EST by Markus Keller CLA
Modified: 2011-12-05 06:09 EST (History)
7 users (show)

See Also:


Attachments
platform bindings v01 (39.57 KB, patch)
2008-10-02 14:08 EDT, Paul Webster CLA
no flags Details | Diff
Revert some of the binding platform change (7.83 KB, patch)
2008-10-03 15:05 EDT, Paul Webster CLA
no flags Details | Diff
Simply honour the carbon bindings on cocoa v03 (2.23 KB, patch)
2008-10-06 09:45 EDT, Paul Webster CLA
no flags Details | Diff
Simply honour the carbon bindings on cocoa v04 (3.50 KB, patch)
2008-10-06 10:44 EDT, Paul Webster CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2008-02-05 06:27:58 EST
I20080205-0010

The org.eclipse.ui.bindings extension point's key element will cause trouble when cocoa becomes a supported platform.

We already have a lot of redundancy in our contributions for redefinitions from M2+M3+* to COMMAND+ALT+* for platform="carbon". If we have to do the same also for platform="cocoa", the plugin.xml becomes really unmanageable.

Possible solutions:

a) allow multiple attributes in the platform attribute, e.g. platform="carbon,cocoa".

b) add a new attribute 'os', which would be compared to the value of org.eclipse.core.runtime.Platform.getOS(), e.g. os="macosx".

c) add something like a sequenceModifier element to the bindings extension point to support transformations like this on a broader level,
e.g. (syntax would have to be polished!):
<extension point="org.eclipse.ui.bindings">
<sequenceModifier
    find="M2+M3"
    replace="COMMAND+ALT"
    platforms="carbon,cocoa"
/>
...
Comment 1 Dani Megert CLA 2008-02-05 07:48:48 EST
Another candidate will be the new WPF version. Not sure whether there will be a win64 as well.
Comment 2 Paul Webster CLA 2008-09-23 10:51:36 EDT
(In reply to comment #0)
> Possible solutions:
> 
> a) allow multiple attributes in the platform attribute, e.g.
> platform="carbon,cocoa".

I'm inclined to implement this solution.  comma separated platforms would need to be provided instead of the single carbon platform, but can easily be used to generate the correct binding objects and the same solution can be applied to windows.


Another suggestion that could also be implemented in addition to the above is to simply honour the carbon bindings on cocoa as well.  The big plus for this is that it simplifies the migration path for everybody downstream.

But some of the drawbacks are 

1) everything from carbon will propagate to cocoa, making it difficult for them to go their separate ways in the future

2) it is not an obvious property of the windowing system "platform" represents (carbon == cocoa?).  This would really more be about creating the os="mac" property and the same for windows, but that would defeat the point of providing a simplified migration path.

3) a similar hack would need to be put in place if we were doing the same thing for win32->wpf, with the same drawbacks.

For now I'm going to go with the platform="carbon,cocoa" and not the second suggestion, but I'm open to other opinions/suggestions.

PW
Comment 3 Markus Keller CLA 2008-09-23 12:53:01 EDT
Solution a) would be sufficient to close this bug (allows us to do a global search/replace to add cocoa, and then proceed as before).

Making cocoa a sub-platform of carbon (and wpf a sub-platform of win32) would be hard to understand, and if you just hardcode this in Platform/UI, then other SWT ports could not use a similar mechanism. I wouldn't do that.

If you could also implement something like solution c), then our plugin.xmls would become even more readable, since most of the exceptions we currently have are replacing M2+M3+* with COMMAND+ALT+* on the Mac. But if you don't find time to do this, I can also copy it into a new enhancement request.
Comment 4 Paul Webster CLA 2008-09-26 13:23:52 EDT
OK, I'll go ahead with solution a) for now.

We can talk about a possible transform enhancement in bug 248752

PW
Comment 5 Paul Webster CLA 2008-10-02 14:08:30 EDT
Created attachment 114124 [details]
platform bindings v01

Allow platform="carbon,cocoa" to be used in keybindings.

PW
Comment 6 Paul Webster CLA 2008-10-02 14:29:04 EDT
Released to HEAD >20081002
PW
Comment 7 Felipe Heidrich CLA 2008-10-02 14:50:53 EDT
(In reply to comment #6)
> Released to HEAD >20081002
> PW

Thank you!
Comment 8 Paul Webster CLA 2008-10-03 12:12:54 EDT
Since we're looking at a transform enhancement in Bug 248752, the value of this change is less than I had hoped.  I talked to Dani, and I think for M3 I'm going to back out this change, and simply honour "carbon" keybindings on "cocoa".

This will allow carbon and cocoa keybindings to work in M3 but the contributors will only have to change their code once in M4.

PW
Comment 9 Paul Webster CLA 2008-10-03 15:05:13 EDT
Created attachment 114218 [details]
Revert some of the binding platform change

This will revert parts of the change related to the new binding persistence but not the Util.split(*) upgrade.

PW
Comment 10 Paul Webster CLA 2008-10-06 09:31:02 EDT
(In reply to comment #9)
> Created an attachment (id=114218) [details]
> Revert some of the binding platform change

Released revert.

PW
Comment 11 Paul Webster CLA 2008-10-06 09:45:04 EDT
Created attachment 114306 [details]
Simply honour the carbon bindings on cocoa v03
Comment 12 Paul Webster CLA 2008-10-06 10:44:54 EDT
Created attachment 114314 [details]
Simply honour the carbon bindings on cocoa v04
Comment 13 Paul Webster CLA 2008-10-06 10:46:36 EDT
(In reply to comment #12)
> Created an attachment (id=114314) [details]
> Simply honour the carbon bindings on cocoa v04
> 

Released to HEAD >20081006
PW
Comment 14 Paul Webster CLA 2008-10-28 13:19:40 EDT
In I20081027-1800
PW