Resending.  Rietveld was misbehaving the first time I sent these, so I
thought I'd retry to get them in a more readable form.


http://codereview.appspot.com/89073/diff/10001/10008
File src/com/google/caja/plugin/domita.js (right):

http://codereview.appspot.com/89073/diff/10001/10008#newcode815
Line 815: if (typeof str !== 'string') return fail;
ok, so str can't change shape since you typecheck

http://codereview.appspot.com/89073/diff/10001/10008#newcode817
Line 817: if (0 < n && str.substring(n) === suffix) {
0 < n should be 0 <= n since str is a suffix of str for all str.

http://codereview.appspot.com/89073/diff/10001/10008#newcode828
Line 828: function virtualizeAttributeValue(attrType, realValue) {
do we need to force realValue to a string or do all clients do that.

http://codereview.appspot.com/89073/diff/10001/10008#newcode837
Line 837: return unsuffix(id, idSuffix, '') + spaces;
can we normalize spaces.  instead of returning
unsuffix(...) + spaces, maybe return unsuffix(...) + ' '.

http://codereview.appspot.com/89073/diff/10001/10008#newcode910
Line 910: function(_, id, spaces) { return id + idSuffix + spaces; });
Same as at 837.

http://codereview.appspot.com/89073/diff/10001/10007
File tests/com/google/caja/plugin/templates/TemplateCompilerTest.java
(right):

http://codereview.appspot.com/89073/diff/10001/10007#newcode553
Line 553: }
Can you add an assertMessagesLessSevereThan(MessageLevel.WARNING) to
the end of the new tests.
If there should be warnings, you can use assertMessage(true, ...) to
check them and remove them from the queue.
I want to make sure that any warnings about sketchy classes stay
visible.

http://codereview.appspot.com/89073

Reply via email to