Community
Participate
Working Groups
It would be nice to have access to the Dialog messages (eg. IDialogConstants.OK_LABEL) trough API instead of a field access. This would allow the RAP team to provide a session-based translation for the labels and furthermore improves the usability for our current users. Developers who want to single-source their applications for RCP and RAP cannot use the existing API of JFace as the labels are hardcoded as static fields and RAP has no chance to intervene this behavior without breaking API. I suggest to provide an accessor like SWT.getMessage() to get the translated string as this would allow developers to use the same API in RCP and JFace. I'd be happy to provide a patch once we decided on a possible approach. As suggested above, I'd introduce an accessor on Dialog itself and introduce message keys in IDialogConstants.
Would be great for single-source. +1
Created attachment 159323 [details] Suggested solution This patch introduces Dialog#getButtonLabel(int). The idea is to pass in one of the button-ID's (e.g.OK_ID) from IDialogConstants and the method will return the translated label. However, there is one obstacle: the details button has two labels (show... and hide...) but there is only one ID. The current approach is to ignore this and always return the "show details" text if the DETAILS_ID is passed in. Can we live with that? The implementation is yet incomplete in that it does not cover all ID's. If this approach is accepted, I will provide the missing implementation.
Sounds good to me. Would make things a lot easier.
Is there anything I can help to make this happen in 3.6? Having such an API would remove one of the biggest remaining pain-points in single-sourcing RCP and RAP applications.
(In reply to comment #4) > Is there anything I can help to make this happen in 3.6? > Having such an API would remove one of the biggest remaining pain-points in > single-sourcing RCP and RAP applications. API freeze is M6 (this week) with the first test pass tomorrow. This request came in at the tail end of M5 with M6 already full (sorry). Are you saying that RCP apps would have to use these methods (instead of the static fields) when implementing their dialogs? If so, can't this helper method be included in some RCP->RAP util class? Because you would require clients to change the normal convention anyway....and if they have to use this method, they may as well use some utility method that you provide. If I'm misunderstanding the intention, and if this were the "last remaining pain point" in single sourcing RCP and RAP, I'd encourage you to clarify the request made in this bug, and then include justification for breaking API freeze that could be brought to the PMC if warranted.
(In reply to comment #5) > [ ... ] > > API freeze is M6 (this week) with the first test pass tomorrow. > This request came in at the tail end of M5 with M6 already full (sorry). > > Are you saying that RCP apps would have to use these methods (instead of the > static fields) when implementing their dialogs? If so, can't this helper method > be included in some RCP->RAP util class? Because you would require clients to > change the normal convention anyway....and if they have to use this method, they > may as well use some utility method that you provide. > > If I'm misunderstanding the intention, and if this were the "last remaining pain > point" in single sourcing RCP and RAP, I'd encourage you to clarify the request > made in this bug, and then include justification for breaking API freeze that > could be brought to the PMC if warranted. RCP apps that use these methods could run without changes on RAP. If running on RAP isn't desired, application code can stay with the static fields. The situation today is that clients use two util classes with the same API but different implementations. These util classes reside in diferent bundles (util.rap and util.rcp) and the matching bundle is deployed with the respective platform. If there were these emthods in JFace (that RAP would implement in a multi-user-aware way) the util bundles would become obsolete. Existing RCP apps would only need to change from static fields to these methods if they whish to run on RCP and RAP. Does that make sense? I am not sure if this request would justify breaking the API freeze. Could you point me to some guides/rules/prior examples?
(In reply to comment #6) > If there were these emthods in JFace (that RAP would implement in a > multi-user-aware way) the util bundles would become obsolete. Existing RCP apps > would only need to change from static fields to these methods if they whish to > run on RCP and RAP. > Does that make sense? Yes, but it seems that you could have some common util "single source" bundle that you could also choose to implement in a multi-user-aware way. It doesn't sound like something that *has* to be in JFace. > > I am not sure if this request would justify breaking the API freeze. Could you > point me to some guides/rules/prior examples? Usually it's something like: - new API was introduced during the release that is broken or needs modification - late-breaking critical bug find that can only be fixed with new API It is rarely a new feature because committed feature requests get planned/worked on early. I think it's best to ping us early in 3.7... Even then, I would be asking the same question - what's the difference between putting these methods in a special rap/rcp bundle vs. JFace?
@Susan: here is the early ping for 3.7 that you requested :-) I am project lead for Riena and we like to run Riena on top of RAP out of the box in the same manor that we do that with RCP today. Getting the OK LABEL in RCP is like IDialogConstants.OK_LABEL (OK_ID is IDialogConstants.OK_ID) In RAP since every user could have a different locale a static is no good. So IDialogConstants.OK_LABEL would mean every user has the same locale. So RAP changed the API to use IDialogConstants.get().OK_LABEL. Still for stuff like OK_ID it uses IDialogConstants.OK_ID. Now we use that in multiple places. And yes we could write a wrapper around JFace IDialogConstants to distinguish between RCP and RAP and then later e4. Which does really make a lot of sense. <ironic/> I dont think the current API of JFace has to change but as an alternative for people who care about locale and single sourcing with RAP (having only one source for RCP and RAP) it would be VERY helpful to embedd the 20 lines of code in RCP that allows you to call IDialogConstants.get().OK_LABEL...... And its not rocket science so the chance of breaking anything is minimal..... I can submit a patch if that makes it happen faster..... :-)
(In reply to comment #8) > @Susan: here is the early ping for 3.7 that you requested :-) thank you. > Now we use that in multiple places. And yes we could write a wrapper around > JFace IDialogConstants to distinguish between RCP and RAP and then later e4. > Which does really make a lot of sense. <ironic/> ;-) > > I dont think the current API of JFace has to change but as an alternative for > people who care about locale and single sourcing with RAP (having only one > source for RCP and RAP) it would be VERY helpful to embedd the 20 lines of code > in RCP that allows you to call > > IDialogConstants.get().OK_LABEL...... > > And its not rocket science so the chance of breaking anything is minimal..... > > I can submit a patch if that makes it happen faster..... :-) Absolutely it will make things happen faster, I'd be happy to review a patch and shepherd it in.
rüdiger already attached a patch..... (I overlooked that earlier) so the ball is in your field :-) (In reply to comment #9) > (In reply to comment #8) > > @Susan: here is the early ping for 3.7 that you requested :-) > > thank you. > > > Now we use that in multiple places. And yes we could write a wrapper around > > JFace IDialogConstants to distinguish between RCP and RAP and then later e4. > > Which does really make a lot of sense. <ironic/> > > ;-) > > > > > I dont think the current API of JFace has to change but as an alternative for > > people who care about locale and single sourcing with RAP (having only one > > source for RCP and RAP) it would be VERY helpful to embedd the 20 lines of code > > in RCP that allows you to call > > > > IDialogConstants.get().OK_LABEL...... > > > > And its not rocket science so the chance of breaking anything is minimal..... > > > > I can submit a patch if that makes it happen faster..... :-) > > Absolutely it will make things happen faster, I'd be happy to review a patch > and shepherd it in.
(In reply to comment #10) > r�diger already attached a patch..... (I overlooked that earlier) > > so the ball is in your field :-) ok, I took that comment to mean you had a different patch in mind...
will address next week early in m4 (slipped through cracks)
will look at these over holidays...
I looked at this patch. I don't really like the fact that button ID's, which are intended to be used for identifying a button in the buttonPressed methods, are instead used as pseudo-keys for the translations. This introduces the fuzziness concerning the details button (mentioned by Rudiger in comment #2). What if we instead simply exposed the JFaceResource keys through API in IDialogConstants and RAP clients were told to use pattern JFaceResources.getString(key); For example IDialogConstants.ABORT_LABEL_KEY IDialogConstants.SHOW_DETAILS_KEY IDialogConstants.HIDE_DETAILS_KEY This also reduces the risk of someone adding an ID and/or label to IDialogConstants and not making the corresponding update to the proposed Dialog method.
(In reply to comment #14) > What if we instead simply exposed the JFaceResource keys through API in > IDialogConstants and RAP clients were told to use pattern > > JFaceResources.getString(key); Thanks for taking care of this issue! Speaking for the RAP team, this suggestion sounds good to us. I'd like to point out that this is not a RAP-only issue, but part of an effort to provide support for multiple locales in the platform (bug 275658, bug 75320). Therefore, I would also suggest to either deprecate the old constants, or to add a pointer to their JavaDoc that JFaceResources.getString(key) should be used instead for multi-locale code.
Fixed in HEAD >20110112. Note that I could not add the new key constants to IDialogConstants because that would be a breaking API change. (If IDialogConstants had been marked @noimplement it would be OK, but it is not.) So I have created a new class, IDialogLabelKeys (marked @noimplement so that we can extend it in the future if needed.) The javadoc for the IDialogConstants static label constants now point the reader to the associated key in IDialogLabelKeys with a discussion of the tradeoff (speed of single lookup vs ability to handle multiple locales). This same tradeoff is discussed in the new key constants in IDialogLabelKeys. If there are any changes anyone would like to see to the documentation, please open a new bug with a patch for suggested changes.
Verified through source inspection, I20110124-1800. Thanks Dani for fixing the copy/paste error in the javadoc.
