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
