Thanks for the review!
For the vendor prefixes, I ended up creating a separate helper class,
VendorPrefixes. It doesn't keep a master list of prefixes. I decided
that was more future-proof, and the "-foo-" syntax is specifically
reserved by the W3C for vendor prefixes:
http://www.w3.org/TR/CSS21/syndata.html#vendor-keywords
http://gwt-code-reviews.appspot.com/1727803/diff/1/user/src/com/google/gwt/resources/css/RtlVisitor.java
File user/src/com/google/gwt/resources/css/RtlVisitor.java (right):
http://gwt-code-reviews.appspot.com/1727803/diff/1/user/src/com/google/gwt/resources/css/RtlVisitor.java#newcode43
user/src/com/google/gwt/resources/css/RtlVisitor.java:43: // TODO: These
could be moved to somewhere common before submitting.
On 2012/06/05 10:57:01, tbroyer wrote:
Into CssProperty maybe?
Obsolete, since I introduced a VendorPrefixes utility.
http://gwt-code-reviews.appspot.com/1727803/diff/1/user/src/com/google/gwt/resources/css/RtlVisitor.java#newcode44
user/src/com/google/gwt/resources/css/RtlVisitor.java:44: // TODO: Can
GWT use Guava libraries? This would be much easier with
On 2012/06/05 10:57:01, tbroyer wrote:
gwt-dev.jar bundles Guava, so you can use Guava from generators.
Thanks for the heads-up! I had tried just "com.google.common", which
wasn't working. I didn't realize it was in a special package. It works
now.
This is actually obsolete too, but I'm now using the Guava Splitter, so
it still helped.
http://gwt-code-reviews.appspot.com/1727803/diff/1/user/src/com/google/gwt/resources/css/RtlVisitor.java#newcode204
user/src/com/google/gwt/resources/css/RtlVisitor.java:204:
swapFour(values);
On 2012/06/05 10:57:01, tbroyer wrote:
Maybe rename swapFour to swapFourBorders?
Good idea. Renamed to swapFourSides instead, since it's used for
padding, margin, etc. too.
http://gwt-code-reviews.appspot.com/1727803/diff/1/user/src/com/google/gwt/resources/css/RtlVisitor.java#newcode370
user/src/com/google/gwt/resources/css/RtlVisitor.java:370: if
(parts.length > 2 && "".equals(parts[0])
On 2012/06/05 10:57:01, tbroyer wrote:
How about transforming the property name before splitting it?
(also, we could fix the locale-sensitive toLowerCase)
name = name.toLowerCase(Locale.ENGLISH);
if (name.charAt(0) == '-') {
int nextHyphen = name.indexOf('-', 1);
if (nextHyphen > 0) {
String prefix = name.substring(1, nextHyphen);
if (VENDOR_PREFIXES.contains(prefix)) {
name = name.substring(nextHyphen + 1);
}
}
}
String[] parts = name.split("-");
Alternatively, we could loop over the known prefixes (with
VENDOR_PREFIXES
containing the "-webkit-", "-o-", etc.):
name = name.toLowerCase(Locale.ENGLISH);
if (name.charAt(0) == '-') {
for (String prefix : VENDOR_PREFIXES) {
if (name.startsWith(prefix)) {
name = name.substring(prefix.length());
break;
}
}
}
String[] parts = name.split("-");
Done.
Did some benchmarking with Caliper. Looping to eliminate the prefix is
indeed faster by a bit. I tried using the Guava Splitter and CaseFormat
on a whim, and the former actually runs quite a bit faster.
benchmark trial us linear runtime
SplitThenSetLookup 0 9.46 =============================
SplitThenSetLookup 1 9.46 ==============================
SplitThenSetLookup 2 9.38 =============================
SetLookupThenSplit 0 8.91 ============================
SetLookupThenSplit 1 8.88 ============================
SetLookupThenSplit 2 8.89 ============================
LoopPrefixesThenSplit 0 8.86 ============================
LoopPrefixesThenSplit 1 8.88 ============================
LoopPrefixesThenSplit 2 8.91 ============================
GuavaSplitter 0 3.79 ============
GuavaSplitter 1 3.84 ============
GuavaSplitter 2 3.83 ============
GuavaCaseFormat 0 7.05 ======================
GuavaCaseFormat 1 7.08 ======================
GuavaCaseFormat 2 7.06 ======================
http://gwt-code-reviews.appspot.com/1727803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
