Community
Participate
Working Groups
At the moment the only convenient API for handling base text direction enforcement is provided for Text widget. This work is done via bug 383185. Additional SWT enabling work for the rest of SWT widgets is being done via bug 273198. The API for Text widget looks like this: public static void applyBidiProcessing(Text field, String handlingType) Where handlingType can be: BidiUtils.LEFT_TO_RIGHT BidiUtils.RIGHT_TO_LEFT BidiUtils.AUTO BidiUtils.BTD_DEFAULT This API should be extended to provide support for additional SWT widgets. For example: public static void applyBidiProcessing(Tooltip context, String handlingType) public static void applyBidiProcessing(Item context, String handlingType) ....
Created attachment 228364 [details] applyBidiProcessing for other SWT controls
Hello Markus, Could you please review this patch ? Thank you
I don't see why we need another API for this. Is Control#setTextDirection(int) not good enough? Note that JFace doesn't wrap all SWT APIs. If the SWT API is convenient enough, then clients should just use that one. I would just add this to BidiUtils#applyBidiProcessing: * @see Control#setTextDirection(int) Also note that the API from bug 273198 comment 22 doesn't allow 0. And if we added this API, we would change the parameter names (field -> control, handlingType -> textDirection) and the method name (to applyTextDirection). Overloading applyBidiProcessing with a method that doesn't support the same arguments is a no-go.
(In reply to comment #3) > I don't see why we need another API for this. Is > Control#setTextDirection(int) not good enough? No. It is NOT good enough. Please notice that applyBidiProcessing is being controlled through command line arguments for specifying base text direction. Thus all plugins / applications using this API will be consistently controlled through the command line. Leveraging Control#setTextDirection does not allow that flexibility and will break this consistency. > Also note that the API from bug 273198 comment 22 doesn't allow 0. And if we > added this API, we would change the parameter names (field -> control, > handlingType -> textDirection) and the method name (to applyTextDirection). > Overloading applyBidiProcessing with a method that doesn't support the same > arguments is a no-go. I asked you several times to stay with different APIs for handling base text direction and structured text. You insisted on having one - applyBidiProcessing - covering both. Now, you are saying we should again split it into 2 ? I don't think it is fare. Structured text is usually being treated either as part of input fields (Text, StyledText) or on the text level. For example, it is not practical to expect all labels in the tree to show file paths. Thus supporting STT for the rest of SWT controls in the same manner it is supported in Text widget does not make any sense. In any case segment listeners are not defined for all SWT widgets and it is currently not possible technically. Even in the context of base text direction itself Control#setTextDirection(int) supports currently only LTR and RTL. It does not support Contextual. However, this is a work in progress and it will be added later on. We do need a tool for consistent addressing the enforcement of base text direction in application for selected widgets when value of base text direction for all such widgets is passed from command line. Any suggestion on how to achieve that would be highly welcomed. If you suggest to introduce applytextDirection method for Text, StyledText and rest of currently supported SWT widgets (in addition to applyBidiProcessing) I am fine with that. PLEASE LET US KNOW YOU DECISION. THANKS :-))).
Created attachment 228411 [details] Introducing the new API BidiUtils.applyTextDirection Hello Markus, Following your suggestions I created a new API BidiUtils.applyTextDirection which handles text direction for SWT controls. Can you please review the changes ? Thank you
Just to clarify on my comment #4 above. From base text direction enforcement perspective 99 % of time the usage of API is going to be: applyTextDirection(myControl, BidiUtils.BTD_DEFAULT) Namely, the invocation and actual value for base text direction are going to come from command line (rather than from explicit value passed to the API). This is the main reason why we need one common API using which we can consistently address enforcement of base text direction in different SWT controls. For more info on command line options please see bug 307307
Created attachment 228425 [details] fix 2 Moshe, could you please review this patch?
Paul, I think we should release comment 7 for the 1pm rebuild.
Created attachment 228426 [details] fix 3 Sorry, the implementation of the fix 2 was broken. This one is better and the code is easier to understand.
Fixed with http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=8c63c035df1cfce99e6c0d8ae4c44c6c3fcd307e
