Summary: | [BIDI] BidiUtils.applyBidiProcessing should work on formatting messages | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] Platform | Reporter: | Tomer Mahlin <tomerm> | ||||||||
Component: | UI | Assignee: | Markus Keller <markus.kell.r> | ||||||||
Status: | NEW --- | QA Contact: | |||||||||
Severity: | enhancement | ||||||||||
Priority: | P3 | CC: | Lina.Kemmel, markus.kell.r, tjwatson, wajnberg | ||||||||
Version: | 4.5 | ||||||||||
Target Milestone: | --- | ||||||||||
Hardware: | PC | ||||||||||
OS: | Windows 7 | ||||||||||
Whiteboard: | |||||||||||
Bug Depends on: | 465495, 467836 | ||||||||||
Bug Blocks: | 229010, 381487 | ||||||||||
Attachments: |
|
Description
Tomer Mahlin
2015-05-04 14:13:52 EDT
Each separate placeholder (as well as the entire message as whole if necessary) can be processed using API described in bug 465495 Which BidiUtils class are you referring to? I suspect this bug does not belong in the framework. org.eclipse.jface.util.BidiUtils JFace is Platform/UI. Moved the bug. Bug 465880 is also about combining multiple bidi-processed strings, but in a slightly different setup. Maybe there's some code-sharing opportunity between the two bugs. (In reply to Markus Keller from comment #4) > JFace is Platform/UI. Moved the bug. > > Bug 465880 is also about combining multiple bidi-processed strings, but in a > slightly different setup. Maybe there's some code-sharing opportunity > between the two bugs. The bug 465880 is about editable control context. API in this bug is working on the string level. Thus there is no any relation besides the fact that Bidi is being handled in both. The only common functionality is usage of low level API injecting UCC into strings. This defect is about handling finite well defined (from programmer perspective) string. Bug 465880 is about potentially infinite sequence of tokens not known to developer in advance. I will add more comments on bug 465880 in the bug thread itself. Most probably the suggested signature will change based on the widely used coding pattern for handling message formatting. For example: MessageFormat.format("Exception in {0}.performCopy(): {1}", new Object[] { getClass().getName(), e.getTargetException() }) Observe that second argument passed to format function is simply an array of objects which hold values to replace placeholders in the message. Thus previously suggested pattern: applyBidiProcessing( String messageID, String[] placeholdersData, String[] bidiHandlingType); will change to: Object[] applyBidiProcessing ( String typeOfContext, Object ...args); - typeOfContext - is used to distinguish between use cases: message with placeholders vs. concatenations. - Object ... args - unlimited number of arguments associated with either placeholders or concatenated strings. It is always even number since for each placeholder / piece of text, we need to specify which type of handling is required (i.e. text direction enforcement, structured text, no handling). 1. Message with placeholders ----------------------------- Sample call: applyBidiProcessing ( BIDITYPE_MESSAGEWITHPLACEHOLDERS, placeholderData_1, BTD_LTR, placeholderData_2, NULL // for numbers placeholderData_3, STT_FILEPATH); 2. Concatenation of strings ---------------------------- Sample call: applyBidiProcessing ( BIDITYPE_CONCATENATION, firstPieceOfText, BTD_LTR, secondPieceOfText, NULL // for numbers thirdPieceOfText, STT_FILEPATH); The main difference is in the type of result returned by the function. In case of messages it is just array of objects, while in case of concatenations it is array of objects with just one object - String. Taking typical example of message formatting from above: MessageFormat.format("Exception in {0}.performCopy(): {1}", new Object[] { getClass().getName(), e.getTargetException() }) to allow proper Bidi processing in this context we will suggest to change it (for relevant context / messages) as follows: MessageFormat.format("Exception in {0}.performCopy(): {1}", applyBidiProcessing( BIDITYPE_MESSAGEWITHPLACEHOLDERS, getClass().getName(),STT_FILEPATH e.getTargetException(),BTD_DEFAULT)) In case of concatenation, we will need to retrieve first object from the result array which will hold the result String. PS. Important observation, number of arguments to be passed to new function will be always odd and will be at least 3. Each placeholder or piece of concatenated string comes with its type. Created attachment 253623 [details] new API applyBidiProcessing for formatting messages Hello Markus, Could you please review this patch ? Actually this patch is for both Bug 465495 and Bug 466345. I think it would be much easier for you to review both defects at the same time. Thank you for your review Created attachment 253627 [details]
new API applyBidiProcessing for formatting messages
Dear @markus, any review comments ? Created attachment 259850 [details]
Unit tests for applyBidiProcessing formatting messages API
Hello @markus,
'
I've created these unit tests to facilitate your review
Please let me know if you need anything else
Thank you very much for your review
Sorry, I didn't get to this in time for M6, but I'll have a look in M7 and ask for PMC approval for the late API addition. Thanks Markus ! Appreciate it very much in advance. Sorry, I have to move this out, just due to lack of time and other higher-priority items. |