Snapshot

* build copies all the necessary libraries to WEB-INF
* myvn creates a .project such that if cajadevs have appengine installed
they can run and debug the playground from within eclipse.


http://codereview.appspot.com/116069/diff/2001/2029
File build.xml (right):

http://codereview.appspot.com/116069/diff/2001/2029#newcode58
build.xml:58: <!-- servlet dir - needs to be called "war" -->
On 2009/10/09 02:04:47, ihab.awad wrote:
It *needs* to be called "war"? Wow. Please add a little explanation of
why --
i.e., is that a requirement of the <dev_appserver> invoked later
on...?

Done.

http://codereview.appspot.com/116069/diff/2001/2018
File src/com/google/caja/demos/playground/client/CajolingService.java
(right):

http://codereview.appspot.com/116069/diff/2001/2018#newcode12
src/com/google/caja/demos/playground/client/CajolingService.java:12:
public interface CajolingService extends RemoteService {
Renamed to PlaygroundService.

On 2009/10/09 02:04:47, ihab.awad wrote:
So this caused a name conflict with The Class Formerly Known As
CajolingService
[TM]. Hmm.... This class is really the main facade for the Playground
to talk
back to its server, so PlaygroundBackendService may not be too
specific.

http://codereview.appspot.com/116069/diff/2001/2018#newcode17
src/com/google/caja/demos/playground/client/CajolingService.java:17: *
@return Returns the cajoled html, js and cajoling messages as an array
On 2009/10/09 02:04:47, ihab.awad wrote:
s/Returns//g.

Also, not clear from this comment that you mean:

   return[0] -> html
   return[1] -> js
   return[2 .. (return.length - 1)] -> messages

Done.

http://codereview.appspot.com/116069/diff/2001/2016
File
src/com/google/caja/demos/playground/client/CajolingServiceAsync.java
(right):

http://codereview.appspot.com/116069/diff/2001/2016#newcode20
src/com/google/caja/demos/playground/client/CajolingServiceAsync.java:20:
*/
On 2009/10/09 02:04:47, ihab.awad wrote:
Doc comment format ... and missing @author.

Done.

http://codereview.appspot.com/116069/diff/2001/2015
File src/com/google/caja/demos/playground/client/Playground.java
(right):

http://codereview.appspot.com/116069/diff/2001/2015#newcode61
src/com/google/caja/demos/playground/client/Playground.java:61: for (int
i=2; i < result.length - 2; i++) {
On 2009/10/09 02:04:47, ihab.awad wrote:
Spaces around "=".

Done.

http://codereview.appspot.com/116069/diff/2001/2019
File src/com/google/caja/demos/playground/client/ui/PlaygroundView.java
(right):

http://codereview.appspot.com/116069/diff/2001/2019#newcode57
src/com/google/caja/demos/playground/client/ui/PlaygroundView.java:57:
On 2009/10/09 02:04:47, ihab.awad wrote:
Extra blank line.

Done.

http://codereview.appspot.com/116069/diff/2001/2019#newcode70
src/com/google/caja/demos/playground/client/ui/PlaygroundView.java:70:
public void selectTab(int index) {
On 2009/10/09 02:04:47, ihab.awad wrote:
At the very least, the indices used here should be named constants
somewhere.

Done.

http://codereview.appspot.com/116069/diff/2001/2019#newcode78
src/com/google/caja/demos/playground/client/ui/PlaygroundView.java:78:
for (Menu menu: Menu.values()) {
On 2009/10/09 02:04:47, ihab.awad wrote:
Space around ":".

Done.

http://codereview.appspot.com/116069/diff/2001/2019#newcode96
src/com/google/caja/demos/playground/client/ui/PlaygroundView.java:96:
logoPanel.add(new
Image("http://cajadores.com/demos/testbed/caja_logo_small.png";));
On 2009/10/09 02:04:47, ihab.awad wrote:
Line wrap.

Done.

http://codereview.appspot.com/116069/diff/2001/2019#newcode108
src/com/google/caja/demos/playground/client/ui/PlaygroundView.java:108:
public void onFocus(FocusEvent event) {
On 2009/10/09 02:04:47, ihab.awad wrote:
Don't you wish Java had concise closures? :)

Done.

http://codereview.appspot.com/116069/diff/2001/2019#newcode165
src/com/google/caja/demos/playground/client/ui/PlaygroundView.java:165:
private DecoratedTabPanel createEditorPanel() {
On 2009/10/09 02:04:47, ihab.awad wrote:
You do realize are dangerously close to implementing the Eclipse UI in
GWT, my
friend.... };->
...all in good time :)

http://codereview.appspot.com/116069/diff/2001/2019#newcode262
src/com/google/caja/demos/playground/client/ui/PlaygroundView.java:262:
System.out.println("Output:<"+prettyPrint(result)+">\n");
Debugging.  Removed.

On 2009/10/09 02:04:47, ihab.awad wrote:
Spaces around "+"

http://codereview.appspot.com/116069/diff/2001/2019#newcode268
src/com/google/caja/demos/playground/client/ui/PlaygroundView.java:268:
}-*/;
On 2009/10/09 02:04:47, ihab.awad wrote:
Wow, native. Cool!

Done.

http://codereview.appspot.com/116069/diff/2001/2019#newcode270
src/com/google/caja/demos/playground/client/ui/PlaygroundView.java:270:
public void setRenderedResult(String result) {
See Playground.html - it was missed from the CL.

On 2009/10/09 02:04:47, ihab.awad wrote:
So ... how do cajita.js and domita.js and whatever else get actually
included in
the GWT-served HTML content? Is there a container HTML page that did
not get
included in this CL?

http://codereview.appspot.com/116069/diff/2001/2019#newcode282
src/com/google/caja/demos/playground/client/ui/PlaygroundView.java:282:
"    var imports = ___.copy(___.sharedImports);\n" +
On 2009/10/09 02:04:47, ihab.awad wrote:
Continuation indent.

Done.

http://codereview.appspot.com/116069/diff/2001/2019#newcode286
src/com/google/caja/demos/playground/client/ui/PlaygroundView.java:286:
"    attachDocumentStub('-g___', { rewrite: function() {return null;} },
imports, gadgetRoot);\n" +
On 2009/10/09 02:04:47, ihab.awad wrote:
Line wrap.

Done.

http://codereview.appspot.com/116069/diff/2001/2019#newcode289
src/com/google/caja/demos/playground/client/ui/PlaygroundView.java:289:
htmlAndJs[1].substring(0, htmlAndJs[1].length() - 9);
On 2009/10/09 02:04:47, ihab.awad wrote:
Perhaps pull the removal of the ending </script> into the regex above,
rather
than hiding it here. :)

Done.

http://codereview.appspot.com/116069/diff/2001/2014
File
src/com/google/caja/demos/playground/server/GWTCajolingServiceImpl.java
(right):

http://codereview.appspot.com/116069/diff/2001/2014#newcode40
src/com/google/caja/demos/playground/server/GWTCajolingServiceImpl.java:40:
implements CajolingService {
On 2009/10/09 02:04:47, ihab.awad wrote:
Indent 2 more spaces.

Done.

http://codereview.appspot.com/116069/diff/2001/2014#newcode52
src/com/google/caja/demos/playground/server/GWTCajolingServiceImpl.java:52:
+ "&mime-type=" + URLEncoder.encode(mimeType, "UTF-8"));
On 2009/10/09 02:04:47, ihab.awad wrote:
We point URIs to the cajoling service? Can we point them back to the
current GWT
server instead?

If that's too much trouble (requiring an entire cajoling service to be
built
into the GWT thingey) then please just add a TODO.

Done.

http://codereview.appspot.com/116069/diff/2001/2014#newcode119
src/com/google/caja/demos/playground/server/GWTCajolingServiceImpl.java:119:
public String fetch(String url) {
No.  Elided.

On 2009/10/09 02:04:47, ihab.awad wrote:
Is this used anywhere?

http://codereview.appspot.com/116069/diff/2001/2012
File src/com/google/caja/service/DefaultCajolingService.java (right):

http://codereview.appspot.com/116069/diff/2001/2012#newcode93
src/com/google/caja/service/DefaultCajolingService.java:93: public void
doPost(HttpServletRequest req, HttpServletResponse resp)
On 2009/10/09 02:04:47, ihab.awad wrote:
Per our discussion, see the doPost I added.

Done.

http://codereview.appspot.com/116069/diff/2001/2002
File src/log4j.properties (right):

http://codereview.appspot.com/116069/diff/2001/2002#newcode1
src/log4j.properties:1: # Configure the console as our one appender
On 2009/10/09 02:04:47, ihab.awad wrote:
This seems identical to the one in third_party/java/appengine/. Was it
your
intention to add it directly to src/ as well, or should this be copied
in via a
build rule?

Removed.

http://codereview.appspot.com/116069

Reply via email to