Expand externs as needed.

Otherwise, the initial namespace will not be considered declared and the compiler will be unhappy.

[mhermanto, 2011-05-03 01:49:47]

Update patch

[johnfargo, 2011-05-03 01:50:17]

http://codereview.appspot.com/4444086/diff/1/java/gadgets/src/main/java/org/a...
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponseBuilder.java
(right):

http://codereview.appspot.com/4444086/diff/1/java/gadgets/src/main/java/org/a...
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponseBuilder.java:242:
if (exp.indexOf(".") <= 0) builder.append("var ");
why include the "=" (granted, it'll never happen)?

http://codereview.appspot.com/4444086/diff/1/java/gadgets/src/main/java/org/a...
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponseBuilder.java:256:
i = index + 1;
if I'm reading this correctly, this expands things from:
foo.bar.baz
to:
[foo, foo.bar, foo.bar.baz]

If so, could we instead use .split("\.")?

ret = new List();
StringBuilder cur = new StringBuilder();
for (String part : value.split("\.")) {
  cur.append(cur.length() > 0 ? "." : "").append(part);
  ret.add(cur.toString());
}
return ret;

http://codereview.appspot.com/4444086/diff/1/java/gadgets/src/test/java/org/a...
File
java/gadgets/src/test/java/org/apache/shindig/gadgets/js/JsResponseBuilderTest.java
(right):

http://codereview.appspot.com/4444086/diff/1/java/gadgets/src/test/java/org/a...
java/gadgets/src/test/java/org/apache/shindig/gadgets/js/JsResponseBuilderTest.java:46:
"var a;\n" +
does this pass? Seems to be missing "= {}"

[mhermanto, 2011-05-03 02:01:35]

Snapshot re-uploaded.

On Mon, May 2, 2011 at 6:50 PM, <johnfargo@gmail.com> wrote:

>
>
>
http://codereview.appspot.com/4444086/diff/1/java/gadgets/src/main/java/org/a...
> File
>
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponseBuilder.java
> (right):
>
>
>
http://codereview.appspot.com/4444086/diff/1/java/gadgets/src/main/java/org/a...
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponseBuilder.java:242:
>
> if (exp.indexOf(".") <= 0) builder.append("var ");
> why include the "=" (granted, it'll never happen)?


Sure. Done.

>


>
>
http://codereview.appspot.com/4444086/diff/1/java/gadgets/src/main/java/org/a...
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponseBuilder.java:256:
> i = index + 1;
> if I'm reading this correctly, this expands things from:
> foo.bar.baz
> to:
> [foo, foo.bar, foo.bar.baz]
>
> If so, could we instead use .split("\.")?
>
> ret = new List();
> StringBuilder cur = new StringBuilder();
> for (String part : value.split("\.")) {
>  cur.append(cur.length() > 0 ? "." : "").append(part);
>  ret.add(cur.toString());
> }
> return ret;
>

Done.

>
>
>
http://codereview.appspot.com/4444086/diff/1/java/gadgets/src/test/java/org/a...
> File
>
>
>
java/gadgets/src/test/java/org/apache/shindig/gadgets/js/JsResponseBuilderTest.java
> (right):
>
>
>
http://codereview.appspot.com/4444086/diff/1/java/gadgets/src/test/java/org/a...
>
>
java/gadgets/src/test/java/org/apache/shindig/gadgets/js/JsResponseBuilderTest.java:46:
> "var a;\n" +
> does this pass? Seems to be missing "= {}"
>
> Uploaded stale snapshot.

>
> http://codereview.appspot.com/4444086/
>

[mhermanto, 2011-05-03 02:07:11]

Addressing John's comments (retry!)

[johnfargo, 2011-05-03 02:20:05]

LGTM

(still :))

On Mon, May 2, 2011 at 7:07 PM, <mhermanto@gmail.com> wrote:

> Addressing John's comments (retry!)
>
>
> http://codereview.appspot.com/4444086/
>