Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(55)

Issue 6782129: Refactor HtmlDefinitions JS emission. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 7 months ago by kpreid2
Modified:
13 years, 7 months ago
Reviewers:
metaweta
CC:
google-caja-discuss_googlegroups.com
Base URL:
http://google-caja.googlecode.com/svn/trunk/
Visibility:
Public.

Description

* Build the quoted-key Closure-compatibility exports into the definition generators, so that there is no list at the end of the file to forget to update. * Merge duplicate code in the two entry points HtmlDefinitions$Builder and HtmlDefinitions.main; the latter now produces output identical to the former (it is not used from code, so I assume it is intended for debugging). @r5168

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+72 lines, -58 lines) Patch
M src/com/google/caja/lang/html/HtmlDefinitions.java View 8 chunks +72 lines, -58 lines 2 comments Download

Messages

Total messages: 2
kpreid2
13 years, 7 months ago (2012-11-28 21:59:34 UTC) #1
metaweta
13 years, 7 months ago (2012-11-29 19:31:11 UTC) #2
LGTM modulo comments.

https://codereview.appspot.com/6782129/diff/1/src/com/google/caja/lang/html/H...
File src/com/google/caja/lang/html/HtmlDefinitions.java (right):

https://codereview.appspot.com/6782129/diff/1/src/com/google/caja/lang/html/H...
src/com/google/caja/lang/html/HtmlDefinitions.java:482: if
(currentDate.indexOf("*/") >= 0) {
I think this is stale code; the stuff below may originally have been in a /* */
delimited comment.  With a single-line comment, we should test for \n as a
terminator.

https://codereview.appspot.com/6782129/diff/1/src/com/google/caja/lang/html/H...
src/com/google/caja/lang/html/HtmlDefinitions.java:583: // TODO(kpreid): This is
duplicative of Builder.build and looks like it
You said this TODO has been done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b