This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
Description
_ tboudreau
2004-07-31 19:03:48 UTC
Re. the 'hidden' attr - I would rather suggest that Mac-specific keybindings be defined in a different layer, installed by some Mac support module. More in keeping with the general usage of layers, IMHO, and cleaner from the perspective of other modifications to the shortcut list. So a provisional -1 from me on that part. Tim I request this issue be split in half as it consists of two entirely different change proposals and it is harder to track when they are mixed together this way. Created attachment 16590 [details]
Diff to support hidden attribute in shortcuts folder
> I would rather suggest that Mac-specific > keybindings be defined in a different layer, installed by some Mac > support module. I agree that would be nicer. AFAIK there is no way to include a module that is provisionally installed, only on the mac. So while I agree with the sentiment, the first goal is to make NetBeans 4.0 something that mac users will want to use. Yes, a module with mac keybindings could be included only in the mac installer; this has some down-sides: - Someone who does a build on a mac is not testing exactly what will ship - Anything that complicates the build/installer build process reduces the likelihood that it will make it into 4.0 - It's another module that references classes it doesn't own (since it will refer to action classes and files in core/ui's layer). That makes it much more likely to get broken, and since we don't have that many people developing on macs, much less likely that the breakage will be found. > Tim I request this issue be split in half as it consists of two > entirely different change proposals and it is harder to track when > they are mixed together this way. Well, either one is pretty useless without the other if the goal is to make NetBeans 4.0 something appealing to mac users. Created attachment 16591 [details]
Diff with the entire set of requested changes
Per Jesse's request, consider this issue split: supporting the "hidden" attribute on ShortcutsFolder can be handled separately in issue 46806. The requested compatibility contract level is "friend". We need to do something for this for 4.0. Agreed that it would be preferable to have a new API for - Registering both editor and system shortcuts in a similar way - Allowing per-platform key mappings but that we're not likely to have that in the 4.0 timeframe, but we do need to create a good user experience for mac users, and the principal blocker is unnatural keyboard shortcuts which are unlike other applications. Created attachment 16592 [details]
Revised patch which also fixes jump/select-next/prev word bindings to be correct for the mac
"there is no way to include a module that is provisionally installed, only on the mac" - not true, actually, you can just have a module which on startup turns itself off if it is not on a Mac. Will not work in case you start a userdir on some other platform and then continue using it on a Mac, though this is probably rare. Currently no way to have a module check for enablement on each startup; issue #26338 would provide essentially the same benefits. Not necessarily convenient in this case however. if we choose to do what Jesse proposed, applemenu would be a good candidate to hold Mac-specific shortcuts Right. May be too much trouble for D, however. Would support a quick fix if it can be removed after D and replaced by something cleaner that we could also use for KB presets, etc. The partial problem with post-promoD is that I still hope that we will be able to keep platform compatible for a while (next promo). Removing any API however does break that hope. So I suggest to do it in a better way. Let's have specific apple shortcuts in applemenu module and let's disable it on other OSes using the issue 46833. <off-topic>
> I still hope that we will be able to keep platform compatible for a while (next promo)
Proper TTV integration (shares renderers w/ propsheet) will require splitting PropertyEnv
into a parent class in org.netbeans.modules.openide.explorer. Should we do it now?
</off topic>
We have a better proposal: Instead of X- to mark things explicit, we will use D in the
shortcut name to indicate that the default menu accelerator key for the platform should be
used. This will map to CTRL or META depending on the platform.
Once I've tested it some more, I will attach a new patch (which also patches many modules'
layer files to use the new designation).
Preferable?
Created attachment 16622 [details]
Revised patch - includes Jarda's patch to conditionally enable modules only on one platform, now uses wildcard character for the default menu accel, converts all necessary keybindings in trunk to use it, moves hiding/modifying keybindings for apple to ap
New patch uses wildcard char, so, for example, DA-F5 resolves on mac to MA-F5 and on pc to CA-F5; modifies the shortcuts editor to, when deleting/replacing, try both and any possible orderings of the modifier characters (AFAICT this was broken before - if you deleted MA-F5 but it was registered as AM-F5, it would come back after a restart). Custom mac keybindings are moved to the applemenu module. I cannot review the changes in editor/** that is up to Mila. Otherwise I am fine with the changes. I require a test for Utilities to cover the behaviour of "D" and a test for ShortcutsFolder to cover the getPermutation method by replacing assigning new a shortcut that forces existing one to be deleted. Then I'd like to remind that we need to update the documentation as well. I wouldn't call it a "wildcard" char, which would mean "any modifier you like". It is a specific, single modifier, which varies strictly with platform. Just a nit about terminology. Please exclude Yarda's quite independent patch from your diff, it is just confusing. Is the following intentional? - <file name="C-0.shadow"> + <file name="C-D.shadow"> <attr name="originalFile" stringvalue="Actions/Window/org-netbeans-core-windows-actions-SwitchToRecentDocumentAction.instance"/> </file> Note s/0/D/. Should be consistent b/w o.o.u.Utilities and o.n.editor.SettingsDefaults as to how Mac OS X is detected: os.name vs. mrj.version? Regarding editor changes we will also need to patch the keybindings xml files so that the shortcuts diffs will be properly stored (the duality of the editor keybindings entering is an unwanted relict from previous versions). In fact we will need to do a similar change like "C"->"D" for system keybindings declarative entering. I need to do some more analysis so I will write more info tomorrow. > Should be consistent b/w o.o.u.Utilities and
> o.n.editor.SettingsDefaults as to how Mac OS X is detected: os.name
> vs. mrj.version?
Last I knew, the editor lib was furiously trying to not have dependencies on org.openide.*.
If that is no longer true, of course, it would be better to use the same code everywhere,
but that's the reason I tested "mrj.version" in the editor lib.
Mila, if you could point out where the settings files are, or supply the necessary diff, I'd
appreciate it!
Mila, I need to get this integrated - any last comments? I do need to find out what xml files you were talking about and what should change there. We need to patch the editor/src/org/netbeans/modules/editor/resources/XMLs/DefaultGlobalKeyBindings.xml and DefaultKeyBindings.xml in the same directory. Your patch seems to override the o.o.u.Utilities.stringToKey() to which we also delegate from our OptionUtilities.stringToKey() when reading the keystrokes from XML file for diffing against the currently present keystrokes. BTW I have entered editor issue 47294 to finally get rid of the keystrokes specifications in the SettingsDefaults. Created attachment 17011 [details]
New version of patches
Mila, I've attached a new version of the patches. There is one discrepancy between the settings XML files and the class SettingsDefaults: In SettingsDefaults, I conditionally set find-next to be meta-G on the mac and F3 everywhere else, and conditionally set select- next-word and friends to use ALT instead of CTRL, to be consistent with other mac applications. However, there is no way to do that in the XML files. We could rev the DTD and add an optional attribute for such a thing; should we do that? You said that the code in SettingsDefaults is actually duplicating what's in the XML file; I don't totally understand how these settings files are used; in practice, what happens is the code in SettingsDefaults.java wins. So I'll need some advice on what to do here - and whenever you do get rid of SettingsDefaults, some way to set keys conditionally by platform will need to be put in so this keeps working. Well I still think that the best solution would be have a dedicated xml file for the mac (and possibly other platforms too if necessary) but that's probably not a short-term solution. I've described how the editor's keybinding xml files are used (and why we want to get rid of the SettingsDefaults) in the issue 47294. Honestly I don't know how to properly solve the Meta-G versus F3 problem nicely. Wouldn't it be possible to have both F3 and Meta-G keybindings defined for it? Not sure whether Alt is not interpreted as Meta on some Unixes or other platforms though (it does not seem to be on my RH9 Linux at least ;). In the worst case we would have to patch the editor keybindings after reading from the xml file so that F3 becomes Meta-G. It can be done e.g. in KeyBindingsMIMEOptionFile at line 120. Dedicated file on macosx? Pretty simple, just provide one in ide/applemenu and make it override the default one provided by editor. Afaik you need applemenu depend on editor to make sure the override always take effect. Binding both F3 and Meta-G is fine - won't cause a problem. But select-next-word and friends should be ctrl on the PC and Alt on the mac. Probably the parsing will just have to be patched for that, or it needs to recognize some special token for "word select modifier" which it can conditionally map to ctrl on the pc and alt on the mac. Not sure I love the idea of making a copy of the editor settings in applemenu - it would be easy for them to get out of sync. But Mila, if I wanted to do that, where should the file go? It's Editors/text/base/Defaults/keybindings.xml. Currently it's like this: <file name="keybindings.xml" url="XMLs/DefaultGlobalKeyBindings.xml"/> Note - if the editor used separate files for each key binding, as core does for global accelerators, there would be little risk of a few Mac-specific bindings in ide/applemenu falling out of synch with editor's defaults, since you would only need to override those you want to change. Okay, I'll attach another patch, which overrides the correct keybindings in its layer. But I think keybinding support is either broken, or having a module supplying only the diff of the available keybindings breaks it (Mila, in our conversation earlier, we concluded that the applemenu module's settings XML file should be a diff of the original. But won't that mean the original editor XML files are completely hidden by the file applemenu adds?). I will test this on an unmodified build, and file separately if it is indeed broken. What happens: - Adding global keybindings works the first time you do it. They are written to the userdir keybindings file. - Removing global keybindings writes nothing to the userdir key bindings file - After shutdown and restart, neither adding nor removing keybindings writes anything to the userdir file Probably in the case of bindings registered with D-whatever, it's looking for a binding M- whatever, and not finding it (it will need to check the D- permutation and the M- permutation on mac or C- permutation elsewhere). Also for removing keybindings, it could simply only be seeing my layer-defined file from applemenu, and not finding anything there to remove. Does the editor actually try to fetch things from various layers? How is the merge done? (and where?) You can find global and user KB settings merging in org.netbeans.modules.editor.options.KeyBindingsMIMEOptionFile Okay, I think I've got it all working and well behaved. Caveats: - When applemenu actually declares a runtime-only dep on editor & core/windows, it silently fails to install. Commented out for now and works without it, though that may be luck. - I've duplicated the "permutations" code in KeyBindingsMIMEOptionFile. This code already has unit tests for the copy in core, as part of the patch. I'm starting to think it might be better to: - Rather than duplicate this code, have a new Utilities method like: public Iterator getPermutations (String keystring); which anyone who was trying to map a given keystroke to a declaratively registered string should use - Utilities.keyToString() doesn't declare ordering, but Utilities.stringToKey() returns only one order. So it's always been true that if you registered SA-Y, keyToString would return AS-Y and you'd find no match. If we think we'll really, soon, redesign all our keyboard shortcut handling, we probably shouldn't bother, as one more layer of indirection would solve this. "When applemenu actually declares a runtime-only dep on editor & core/windows, it silently fails to install." - if there is a bug in the module system, file it, don't grumble. Created attachment 17233 [details]
Yet another diff
If no further comments, I plan to integrate the most recent diff this week. *** Issue 46904 has been marked as a duplicate of this issue. *** Integrated. Checking in core/execution/src/org/netbeans/core/execution/resources/layer.xml; /cvs/core/execution/src/org/netbeans/core/execution/resources/layer.xml,v <-- layer.xml new revision: 1.21; previous revision: 1.20 done Processing log script arguments... More commits to come... Checking in core/favorites/src/org/netbeans/modules/favorites/resources/layer.xml; /cvs/core/favorites/src/org/netbeans/modules/favorites/resources/layer.xml,v <-- layer.xml new revision: 1.8; previous revision: 1.7 done Processing log script arguments... More commits to come... Checking in core/output/src/org/netbeans/core/output/resources/layer.xml; /cvs/core/output/src/org/netbeans/core/output/resources/layer.xml,v <-- layer.xml new revision: 1.8; previous revision: 1.7 done Processing log script arguments... More commits to come... Checking in core/output2/src/org/netbeans/core/output2/layer.xml; /cvs/core/output2/src/org/netbeans/core/output2/layer.xml,v <-- layer.xml new revision: 1.8; previous revision: 1.7 done Processing log script arguments... More commits to come... Checking in core/src/org/netbeans/core/ShortcutsFolder.java; /cvs/core/src/org/netbeans/core/ShortcutsFolder.java,v <-- ShortcutsFolder.java new revision: 1.20; previous revision: 1.19 done Processing log script arguments... More commits to come... RCS file: /cvs/core/test/unit/src/org/netbeans/core/ShortcutsFolderTest.java,v done Checking in core/test/unit/src/org/netbeans/core/ShortcutsFolderTest.java; /cvs/core/test/unit/src/org/netbeans/core/ShortcutsFolderTest.java,v <-- ShortcutsFolderTest.java initial revision: 1.1 done Processing log script arguments... More commits to come... Checking in core/ui/src/org/netbeans/core/ui/MenuWarmUpTask.java; /cvs/core/ui/src/org/netbeans/core/ui/MenuWarmUpTask.java,v <-- MenuWarmUpTask.java new revision: 1.11; previous revision: 1.10 done Processing log script arguments... More commits to come... Checking in core/ui/src/org/netbeans/core/ui/resources/layer.xml; /cvs/core/ui/src/org/netbeans/core/ui/resources/layer.xml,v <-- layer.xml new revision: 1.74; previous revision: 1.73 done Processing log script arguments... More commits to come... Checking in core/windows/src/org/netbeans/core/windows/resources/layer.xml; /cvs/core/windows/src/org/netbeans/core/windows/resources/layer.xml,v <-- layer.xml new revision: 1.34; previous revision: 1.33 done Processing log script arguments... More commits to come... Checking in debuggerjpda/ui/src/org/netbeans/modules/debugger/jpda/resources/mf- layer.xml; /cvs/debuggerjpda/ui/src/org/netbeans/modules/debugger/jpda/resources/mf- layer.xml,v <-- mf-layer.xml new revision: 1.23; previous revision: 1.22 done Processing log script arguments... More commits to come... Checking in editor/libsrc/org/netbeans/editor/SettingsDefaults.java; /cvs/editor/libsrc/org/netbeans/editor/SettingsDefaults.java,v <-- SettingsDefaults.java new revision: 1.39; previous revision: 1.38 done Processing log script arguments... More commits to come... Checking in editor/libsrc/org/netbeans/editor/ext/ExtSettingsDefaults.java; /cvs/editor/libsrc/org/netbeans/editor/ext/ExtSettingsDefaults.java,v <-- ExtSettingsDefaults.java new revision: 1.35; previous revision: 1.34 done Processing log script arguments... More commits to come... Checking in editor/libsrc/org/netbeans/editor/ext/java/JavaSettingsDefaults.java; /cvs/editor/libsrc/org/netbeans/editor/ext/java/JavaSettingsDefaults.java,v <-- JavaSettingsDefaults.java new revision: 1.27; previous revision: 1.26 done Processing log script arguments... More commits to come... Checking in editor/src/org/netbeans/modules/editor/java/NbJavaSettingsInitializer.java; /cvs/editor/src/org/netbeans/modules/editor/java/NbJavaSettingsInitializer.java,v <-- NbJavaSettingsInitializer.java new revision: 1.31; previous revision: 1.30 done Processing log script arguments... More commits to come... Checking in editor/src/org/netbeans/modules/editor/options/ KeyBindingsMIMEOptionFile.java; /cvs/editor/src/org/netbeans/modules/editor/options/KeyBindingsMIMEOptionFile.java,v <-- KeyBindingsMIMEOptionFile.java new revision: 1.9; previous revision: 1.8 done Processing log script arguments... More commits to come... Checking in editor/src/org/netbeans/modules/editor/resources/XMLs/ DefaultGlobalKeyBindings.xml; /cvs/editor/src/org/netbeans/modules/editor/resources/XMLs/ DefaultGlobalKeyBindings.xml,v <-- DefaultGlobalKeyBindings.xml new revision: 1.11; previous revision: 1.10 done Checking in editor/src/org/netbeans/modules/editor/resources/XMLs/ DefaultKeyBindings.xml; /cvs/editor/src/org/netbeans/modules/editor/resources/XMLs/DefaultKeyBindings.xml,v <-- DefaultKeyBindings.xml new revision: 1.11; previous revision: 1.10 done Processing log script arguments... More commits to come... Checking in form/src/org/netbeans/modules/form/resources/layer.xml; /cvs/form/src/org/netbeans/modules/form/resources/layer.xml,v <-- layer.xml new revision: 1.78; previous revision: 1.77 done Processing log script arguments... More commits to come... Checking in i18n/src/org/netbeans/modules/i18n/Layer.xml; /cvs/i18n/src/org/netbeans/modules/i18n/Layer.xml,v <-- Layer.xml new revision: 1.23; previous revision: 1.22 done Processing log script arguments... More commits to come... Checking in ide/applemenu/nbproject/project.xml; /cvs/ide/applemenu/nbproject/project.xml,v <-- project.xml new revision: 1.3; previous revision: 1.2 done Processing log script arguments... More commits to come... RCS file: /cvs/ide/applemenu/src/org/netbeans/modules/applemenu/ DefaultGlobalKeyBindings.xml,v done Checking in ide/applemenu/src/org/netbeans/modules/applemenu/ DefaultGlobalKeyBindings.xml; /cvs/ide/applemenu/src/org/netbeans/modules/applemenu/ DefaultGlobalKeyBindings.xml,v <-- DefaultGlobalKeyBindings.xml initial revision: 1.1 done Checking in ide/applemenu/src/org/netbeans/modules/applemenu/layer.xml; /cvs/ide/applemenu/src/org/netbeans/modules/applemenu/layer.xml,v <-- layer.xml new revision: 1.2; previous revision: 1.1 done Processing log script arguments... More commits to come... Checking in java/src/org/netbeans/modules/java/resources/mf-layer.xml; /cvs/java/src/org/netbeans/modules/java/resources/mf-layer.xml,v <-- mf-layer.xml new revision: 1.51; previous revision: 1.50 done Processing log script arguments... More commits to come... Checking in junit/src/org/netbeans/modules/junit/resources/layer.xml; /cvs/junit/src/org/netbeans/modules/junit/resources/layer.xml,v <-- layer.xml new revision: 1.23; previous revision: 1.22 done Processing log script arguments... More commits to come... Checking in monitor/src/org/netbeans/modules/web/monitor/resources/layer.xml; /cvs/monitor/src/org/netbeans/modules/web/monitor/resources/layer.xml,v <-- layer.xml new revision: 1.15; previous revision: 1.14 done Processing log script arguments... More commits to come... Checking in openide/src/org/openide/util/Utilities.java; /cvs/openide/src/org/openide/util/Utilities.java,v <-- Utilities.java new revision: 1.144; previous revision: 1.143 done Processing log script arguments... More commits to come... Checking in projects/projectui/src/org/netbeans/modules/project/ui/resources/ layer.xml; /cvs/projects/projectui/src/org/netbeans/modules/project/ui/resources/layer.xml,v <-- layer.xml new revision: 1.39; previous revision: 1.38 done Processing log script arguments... More commits to come... Checking in tasklist/compiler/src/org/netbeans/modules/tasklist/compiler/mf-layer.xml; /cvs/tasklist/compiler/src/org/netbeans/modules/tasklist/compiler/mf-layer.xml,v <-- mf-layer.xml new revision: 1.21; previous revision: 1.20 done Processing log script arguments... More commits to come... Checking in tasklist/docscan/src/org/netbeans/modules/tasklist/docscan/mf-layer.xml; /cvs/tasklist/docscan/src/org/netbeans/modules/tasklist/docscan/mf-layer.xml,v <-- mf-layer.xml new revision: 1.25; previous revision: 1.24 done Processing log script arguments... More commits to come... Checking in utilities/src/org/netbeans/modules/utilities/Layer.xml; /cvs/utilities/src/org/netbeans/modules/utilities/Layer.xml,v <-- Layer.xml new revision: 1.67; previous revision: 1.66 done Processing log script arguments... More commits to come... Checking in vcscore/src/org/netbeans/modules/vcscore/resources/mf-layer.xml; /cvs/vcscore/src/org/netbeans/modules/vcscore/resources/mf-layer.xml,v <-- mf- layer.xml new revision: 1.44; previous revision: 1.43 done Processing log script arguments... More commits to come... Checking in workspaceswitcher/navigator/src/org/netbeans/modules/navigator/ layer.xml; /cvs/workspaceswitcher/navigator/src/org/netbeans/modules/navigator/layer.xml,v <-- layer.xml new revision: 1.12; previous revision: 1.11 done Just a note for those of you who are watching this issue with an eye to a usable netbeans on osX, these changes didn't make it into 4.0 beta 2, so you'll have to wait a bit longer. more info at http://weblogs.java.net/pub/wlg/1830 |