On 2009/12/03 01:57:40, MikeSamuel wrote:
LGTM with changes below.

So we should use SomethingWidgyHappened* in regular code so that
people looking
through logs know that the problem originated with Caja code.

But that's not as important for test code, right?

Right.  I got a bit carried away but its definitely not necessary for
tests/.


http://codereview.appspot.com/129056/diff/6054/7020
File src/com/google/caja/lexer/escaping/Escaping.java (right):

http://codereview.appspot.com/129056/diff/6054/7020#newcode92
src/com/google/caja/lexer/escaping/Escaping.java:92: "StringBuilders
don't throw
IOException");
, ex

http://codereview.appspot.com/129056/diff/6054/7020#newcode129
src/com/google/caja/lexer/escaping/Escaping.java:129: "StringBuilders
don't
throw IOException");
, ex

http://codereview.appspot.com/129056/diff/6054/7020#newcode166
src/com/google/caja/lexer/escaping/Escaping.java:166: "StringBuilders
don't
throw IOException");
indent
, ex

http://codereview.appspot.com/129056/diff/6054/7020#newcode194
src/com/google/caja/lexer/escaping/Escaping.java:194: "StringBuilders
don't
throw IOException");
, ex

http://codereview.appspot.com/129056/diff/6054/7020#newcode216
src/com/google/caja/lexer/escaping/Escaping.java:216: "StringBuilders
don't
throw IOException");
, ex

http://codereview.appspot.com/129056/diff/6054/7020#newcode245
src/com/google/caja/lexer/escaping/Escaping.java:245: "StringBuilders
don't
throw IOException");
, ex

http://codereview.appspot.com/129056/diff/6054/7020#newcode532
src/com/google/caja/lexer/escaping/Escaping.java:532: "StringBuilders
don't
throw IOException");
, ex

http://codereview.appspot.com/129056/diff/6054/7035
File src/com/google/caja/parser/Normalizer.java (right):

http://codereview.appspot.com/129056/diff/6054/7035#newcode78
src/com/google/caja/parser/Normalizer.java:78: "Normalizer
unexpectedly
uninvokable", (RuntimeException) th);
Can't we just propagate th up here and in the else clause?
If not, we don't need to branch.

http://codereview.appspot.com/129056/diff/6054/7064
File src/com/google/caja/reporting/Message.java (right):

http://codereview.appspot.com/129056/diff/6054/7064#newcode82
src/com/google/caja/reporting/Message.java:82: throw e;
These three lines can be condensed to 1.



http://codereview.appspot.com/129056

Reply via email to