Different estimation heuristics may be needed for different use cases. For example, for a short input box where the direction behavior should be clear, the first-strong heuristic is generally preferable (it is used, for example, in Google.co.il). For long paragraphs the word-count is better since it's more accurate. In Ads they prefer the any-RTL heuristic.
http://gwt-code-reviews.appspot.com/338801/diff/1/4 File user/src/com/google/gwt/i18n/shared/DirectionEstimator.java (right): http://gwt-code-reviews.appspot.com/338801/diff/1/4#newcode32 user/src/com/google/gwt/i18n/shared/DirectionEstimator.java:32: * Estimates the direction of a string, assuming it's clean (doesn't contain On 2010/04/11 16:13:46, jat wrote:
Instead of "clean", how about:
Estimates the direction of a plain-text string.
Done. http://gwt-code-reviews.appspot.com/338801/diff/1/4#newcode44 user/src/com/google/gwt/i18n/shared/DirectionEstimator.java:44: * @param isHtml Whether {...@code str} is HTML / HTML-escaped. On 2010/04/11 16:13:46, jat wrote:
Should clarify that it is plain text if isHtml is false.
Done. http://gwt-code-reviews.appspot.com/338801/diff/1/6 File user/src/com/google/gwt/i18n/shared/HasDirectionEstimator.java (right): http://gwt-code-reviews.appspot.com/338801/diff/1/6#newcode26 user/src/com/google/gwt/i18n/shared/HasDirectionEstimator.java:26: DirectionEstimator getDirectionEstimator(); On 2010/04/11 16:13:46, jat wrote:
Add @return.
Done, though I thought that for a simple getter this is not needed. http://gwt-code-reviews.appspot.com/338801/diff/1/6#newcode37 user/src/com/google/gwt/i18n/shared/HasDirectionEstimator.java:37: void setDirectionEstimator(boolean enabled); On 2010/04/11 16:13:46, jat wrote:
Add @param enabled.
Done. http://gwt-code-reviews.appspot.com/338801/diff/1/6#newcode42 user/src/com/google/gwt/i18n/shared/HasDirectionEstimator.java:42: * For {...@code null} input, this should turn off direction estimation. On 2010/04/11 16:13:46, jat wrote:
Make this an @param entry.
Done. http://gwt-code-reviews.appspot.com/338801/diff/1/7 File user/src/com/google/gwt/i18n/shared/WordCountDirectionEstimator.java (right): http://gwt-code-reviews.appspot.com/338801/diff/1/7#newcode28 user/src/com/google/gwt/i18n/shared/WordCountDirectionEstimator.java:28: private static final WordCountDirectionEstimator instance = On 2010/04/11 16:13:46, jat wrote:
GWT style is to use all-caps INSTANCE here.
Done. http://gwt-code-reviews.appspot.com/338801/diff/1/9 File user/test/com/google/gwt/i18n/shared/AnyRtlDirectionEstimatorTest.java (right): http://gwt-code-reviews.appspot.com/338801/diff/1/9#newcode25 user/test/com/google/gwt/i18n/shared/AnyRtlDirectionEstimatorTest.java:25: private static AnyRtlDirectionEstimator estimator = On 2010/04/11 16:13:46, jat wrote:
Would it be worth moving estimator to DirectionEstimatorTestBase (set
in a super
constructor call) and make each individual test just a set of expected
results
for various strings?
I don't realize what it would save - we still should have all types of estimators, one for each subclass. On the other hand, it'd impair readability. http://gwt-code-reviews.appspot.com/338801/diff/1/9#newcode28 user/test/com/google/gwt/i18n/shared/AnyRtlDirectionEstimatorTest.java:28: private String containsRtlChar = enWord + digitsWord + wordWithOneRtlChar; On 2010/04/11 16:13:46, jat wrote:
final
Done. http://gwt-code-reviews.appspot.com/338801/diff/1/10 File user/test/com/google/gwt/i18n/shared/DirectionEstimatorTestBase.java (right): http://gwt-code-reviews.appspot.com/338801/diff/1/10#newcode25 user/test/com/google/gwt/i18n/shared/DirectionEstimatorTestBase.java:25: protected String enWord = " abc"; On 2010/04/11 16:13:46, jat wrote:
Make these final and named like EN_WORD?
Done. http://gwt-code-reviews.appspot.com/338801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors To unsubscribe, reply using "remove me" as the subject.
