Json Concat Js throws error while rendering in IE

HTML pages containing json Concat Js throws error "Expression, variable excepted" while rending in IE. This error is due to extra comma present in JSON map in concat javascript. For e.g-> Currently json map looks like 
json = {"http://url/1.js":"var foo=1",}
Because of comma present at last, IE throws error. After applying this patch, json map looks like,
json = {"http://url/1.js":"var foo=1"}
Extra comma is removed.

[pulkitgoyal2000, 2011-03-08 13:54:53]

Refactoring Code

[satya3656, 2011-03-08 17:53:35]

LGTM

[satya3656, 2011-03-08 17:54:40]

On 2011/03/08 17:53:35, satya3656 wrote:
> LGTM

Add @dev to the reviewer list, and ask gagan to review/commit the change.

[gagan.goku, 2011-03-08 19:07:39]

lgtm.
Send out to dev@ and also include a sample snippet of failing js in the cl
description.

http://codereview.appspot.com/4257060/diff/3001/java/gadgets/src/main/java/or...
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java
(right):

http://codereview.appspot.com/4257060/diff/3001/java/gadgets/src/main/java/or...
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java:348:
private boolean objectAdded;
plz rename to something like firstEntry, something that tells you that this is
the first entry to be added or not.
Also initialize to false.

[pulkitgoyal2000, 2011-03-09 05:08:43]

http://codereview.appspot.com/4257060/diff/3001/java/gadgets/src/main/java/or...
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java
(right):

http://codereview.appspot.com/4257060/diff/3001/java/gadgets/src/main/java/or...
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java:348:
private boolean objectAdded;
On 2011/03/08 19:07:39, gagan.goku wrote:
> plz rename to something like firstEntry, something that tells you that this is
> the first entry to be added or not.
> Also initialize to false.

Done.

[pulkitgoyal2000, 2011-03-09 05:10:56]

+dev,ziv,fagro

[anupama.dutta, 2011-03-09 07:49:33]

Good find!
LGTM.

http://codereview.appspot.com/4257060/diff/9001/java/gadgets/src/main/java/or...
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java
(right):

http://codereview.appspot.com/4257060/diff/9001/java/gadgets/src/main/java/or...
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java:348:
private boolean firstEntry;
Minor comment: The name "firstEntry" seems to be used in exactly the opposite
way. It is true for every call of outputJs other than when we actually encounter
the first entry :) So you could consider renaming it to firstEntryOver or
firstEntryDone or something on similar lines.

[gagan.goku, 2011-03-09 12:08:47]

Or set firstEntry as true initially and set to false once you have processed
it later.

On Wed, Mar 9, 2011 at 1:19 PM, <anupama.dutta@gmail.com> wrote:

> Good find!
> LGTM.
>
>
>
>
http://codereview.appspot.com/4257060/diff/9001/java/gadgets/src/main/java/or...
>
> File
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java
> (right):
>
>
>
http://codereview.appspot.com/4257060/diff/9001/java/gadgets/src/main/java/or...
>
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java:348:
> private boolean firstEntry;
> Minor comment: The name "firstEntry" seems to be used in exactly the
> opposite way. It is true for every call of outputJs other than when we
> actually encounter the first entry :) So you could consider renaming it
> to firstEntryOver or firstEntryDone or something on similar lines.
>
>
> http://codereview.appspot.com/4257060/
>

[pulkitgoyal2000, 2011-03-09 12:31:27]

http://codereview.appspot.com/4257060/diff/9001/java/gadgets/src/main/java/or...
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java
(right):

http://codereview.appspot.com/4257060/diff/9001/java/gadgets/src/main/java/or...
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java:348:
private boolean firstEntry;
On 2011/03/09 07:49:33, anupama.dutta wrote:
> Minor comment: The name "firstEntry" seems to be used in exactly the opposite
> way. It is true for every call of outputJs other than when we actually
encounter
> the first entry :) So you could consider renaming it to firstEntryOver or
> firstEntryDone or something on similar lines.

I set firstEntry as true initially and set to false once it is processed.

[gagan.goku, 2011-03-09 13:23:21]

Build looks good.
Committed as r1079785.