Community
Participate
Working Groups
The commands "Toggle Split Editor (Vertical)" and "Toggle Split Editor (Horizontal)" appear under the same name "Toggle Split Editor" in the Window > Editor menu. In Luna the command names were different.
These parameterized org.eclipse.ui.window.splitEditor commands are added to the menu in org.eclipse.ui.ide.application/plugin.xml. The contribution has not been touched. The bug must be somewhere in the rendering of parameterized commands (parameter is lost now).
This is caused by bug 461026 where HandledContributionItem#updateMenuItem() no longer calls ParameterizedCommand#getName(), which would return the name with a parameterization. Dirk, my thoughts are that either HandleItemImpl should implement getLocalizedLabel(), and do the right thing, or LocalizationHelper should have some special support for MHandledItems. I don't know what the right solution is for handling localization of values from an IParameterValues instance.
IMHO the correct solution would be to implement the same logic in ParameterizedCommand#getName() to HandledContributionItem, so we don't need to mix up the concepts. Unfortunately the possible values are lost when transforming the Eclipse 3 command to the application model. In short, the application does not know about the possible parameter values as specified in commandParameter#values. As long as we have this mix up, we either don't support locale changes at runtime or no parameterized menu items. A quick fix could be to add a method ParameterizedCommand#getName(String), where the parameter could be the base name (e.g. command.getName()). This would at least fix the parameterized menu items. Only the locale change at runtime would not work for the parameters. For SR1 I think this could be a suitable workaround. The correct solution should be to introduce the values in the application model aswell. So it could be transferred and inspected in the application model correctly. Any thoughts on that? The first change could be applied quite easily if introducing new public methods is allowed for SR1. The second modification looks like some bigger change as the application model has to evolve. So I think that is nothing for SR1.
New Gerrit change created: https://git.eclipse.org/r/53180
(In reply to Brian de Alwis from comment #2) > Dirk, my thoughts are that either HandleItemImpl should implement > getLocalizedLabel(), and do the right thing, or LocalizationHelper should > have some special support for MHandledItems. I don't know what the right > solution is for handling localization of values from an IParameterValues > instance. I'm against adding special support for MHandledItems in LocalizationHelper. Creating a getLocalizedLabel() for the impls seem to be the right choice. But as I said in the previous comment, we would need the ParameterValues in the application model so we can remove the inspection of ParamterizedCommand. The patch I provided now is the workaround that might be suitable for SR1.
Commands do have MCommandParameters, but we don't do two-way synchronizing of MCommands to the command framework.
(In reply to Brian de Alwis from comment #6) > Commands do have MCommandParameters, but we don't do two-way synchronizing > of MCommands to the command framework. But we don't have the parameter values or am I missing something. We would need those in order to also localize those values.
(In reply to Dirk Fauth from comment #7) > (In reply to Brian de Alwis from comment #6) > > Commands do have MCommandParameters, but we don't do two-way synchronizing > > of MCommands to the command framework. > > But we don't have the parameter values or am I missing something. We would > need those in order to also localize those values. You're right. The real problem here is the IParameterValues whites independent of the localization framework.
The more I think about this the more I wonder. Why do we use such a heavy-weight mechanism to automatically calculate the menu label via parameters when it would be easier, less error prone and potentially less time consuming if we would specify the correct label in the contribution (extension point or application model)? What I mean is, do we really need such a mechanism that calculates the label dependent on the parameters or could we switch to specifying the correct label at the contribution itself?
Any opinions on this? I personally would prefer to simply specify the correct menu item labels in the plugin.xml rather than modifying the code. I personally never used the automatic way of determining the menu item label via parameter values. But I'm not sure how many people are aware of that feature and are using it. As it is a regression we should fix it for 4.5.1, especially because possible solutions are identified. I just need some opinions on the solution to merge.
The actual arguments for parameterized commands are not always known upfront. The menuContribution for the splitEditor command could be hacked to hardcode labels. But this doesn't work in general. See e.g. the org.eclipse.ui.views.showView command, whose parameters are computed dynamically in org.eclipse.ui.internal.registry.ViewParameterValues. Since the org.eclipse.ui.menus extension point is API, requiring clients to specify a label for each parameterized command contribution would be a breaking change.
(In reply to Markus Keller from comment #11) > The actual arguments for parameterized commands are not always known upfront. > > The menuContribution for the splitEditor command could be hacked to hardcode > labels. But this doesn't work in general. See e.g. the > org.eclipse.ui.views.showView command, whose parameters are computed > dynamically in org.eclipse.ui.internal.registry.ViewParameterValues. > > Since the org.eclipse.ui.menus extension point is API, requiring clients to > specify a label for each parameterized command contribution would be a > breaking change. Good point. Therefore my Gerrit patch should be reviewed, as for me this seems to be the way to go for the dynamic approach.
What if we added an IParameterValues2 (or ITranslatableParameterValues) that somehow provided support for translation?
(In reply to Brian de Alwis from comment #13) > What if we added an IParameterValues2 (or ITranslatableParameterValues) that > somehow provided support for translation? Don't for get that you now have java8 so you can easily add a a default method like interface IParameterValue { String getName(); default String getLocalizedName() { return getName(); } }
(In reply to Brian de Alwis from comment #13) > What if we added an IParameterValues2 (or ITranslatableParameterValues) that > somehow provided support for translation? The missing part regarding the new localization mechanism is the ability to change the label value at runtime. Therefore, on changing the locale at runtime, the parameter values won't change. AFAIK the IParameterValues are not part of the application model and there is no representation in Eclipse 4 for that. Please correct me if I'm wrong, but I didn't find anything. Since Eclipse 3 doesn't support locale changes at runtime, I don't know what and how you would achieve providing the translation support. Which in fact is the support for locale changes at runtime.
I'm wondering if there's any value to providing an alternative form of IParameterValues that could use the TranslationService and Do The Right Thing.
(In reply to Brian de Alwis from comment #16) > I'm wondering if there's any value to providing an alternative form of > IParameterValues that could use the TranslationService and Do The Right > Thing. Since the IParameterValue is implemented by a user, it seems to be the users choice whether to support locale changes at runtime or not. The framework expects the value to use. A change would imply that a user needs to specify translation keys. In short, I don't see a reason for adding such a support, nor do I really see a way of doing this.
Gerrit change https://git.eclipse.org/r/53180 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=14efff7cade7722d24e25afc451676f3513280de
(In reply to Eclipse Genie from comment #18) > Gerrit change https://git.eclipse.org/r/53180 was merged to [master]. > Commit: > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/ > ?id=14efff7cade7722d24e25afc451676f3513280de Thanks Dirk. I will test with tomorrows I-Build.
Hi Lars, did the test succeed and can this perhaps be marked Fixed ? CQ:WIND00-WB4-6171
(In reply to Martin Oberhuber from comment #20) > Hi Lars, did the test succeed and can this perhaps be marked Fixed ? > > CQ:WIND00-WB4-6171 Thanks for the reminder Martin. In Build id: N20150927-2000 I see different names, so this is fixed. @Dirk, should the commands also be available in the Quick Access? I do not see them there. If that was not part of this fix, I think we can go ahead and downport this on to 4.5.2.
> @Dirk, should the commands also be available in the Quick Access? I do not > see them there. If that was not part of this fix, I think we can go ahead > and downport this on to 4.5.2. I am not aware of anything regarding the Quick Access. The contributed fix just ensures that the parameter values are added again.
+1 from component lead for downport. @Dirk you need another committer to review this for the downport.
*** Bug 486258 has been marked as a duplicate of this bug. ***
Too late for 4.5.2.
