Updated few checks in ConcatVisitor, to increase its coverage.

Optimization:
Updated few checks in ConcatVisitor, to increase its coverage. 

In getNextSibling() function:
1) We can be ignore are text node irrespective of their content.
2) We can ignore comment nodes expect for conditional comments.

[satya3656, 2010-08-31 13:49:37]

Fixed some comments

[gagan.goku, 2010-09-01 02:56:12]

looking good. Nice change

http://codereview.appspot.com/2085042/diff/4001/5001
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java
(right):

http://codereview.appspot.com/2085042/diff/4001/5001#newcode312
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java:312:
if (commentText.startsWith("[if") || commentText.startsWith("[ if")) {
is "[   if" invalid ?

[satya3656, 2010-09-20 08:47:27]

http://codereview.appspot.com/2085042/diff/4001/java/gadgets/src/main/java/or...
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java
(right):

http://codereview.appspot.com/2085042/diff/4001/java/gadgets/src/main/java/or...
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java:312:
if (commentText.startsWith("[if") || commentText.startsWith("[ if")) {
On 2010/09/01 02:56:12, gagan.goku wrote:
> is "[   if" invalid ?

only '[if' is valid updated the condition

[gagan.goku, 2010-09-28 09:53:02]

lgtm

http://codereview.appspot.com/2085042/diff/12001/java/gadgets/src/main/java/o...
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java
(right):

http://codereview.appspot.com/2085042/diff/12001/java/gadgets/src/main/java/o...
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java:251:
if (cur.getNodeType() == Node.TEXT_NODE) {
Plz rewrite like this:
if (cur.getNodeType() == Node.TEXT_NODE ||
    (cur.getNodeType() == Node.COMMENT_NODE && !isConditionalComment(cur))) {
  continue;
}

http://codereview.appspot.com/2085042/diff/12001/java/gadgets/src/main/java/o...
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java:316:
if (commentText.startsWith("[if")) {
return commentText.startsWith("[if");

[satya3656, 2010-10-04 12:20:58]

http://codereview.appspot.com/2085042/diff/12001/java/gadgets/src/main/java/o...
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java
(right):

http://codereview.appspot.com/2085042/diff/12001/java/gadgets/src/main/java/o...
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java:251:
if (cur.getNodeType() == Node.TEXT_NODE) {
On 2010/09/28 09:53:02, gagan.goku wrote:
> Plz rewrite like this:
> if (cur.getNodeType() == Node.TEXT_NODE ||
>     (cur.getNodeType() == Node.COMMENT_NODE && !isConditionalComment(cur))) {
>   continue;
> }

Done.

http://codereview.appspot.com/2085042/diff/12001/java/gadgets/src/main/java/o...
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java:316:
if (commentText.startsWith("[if")) {
On 2010/09/28 09:53:02, gagan.goku wrote:
> return commentText.startsWith("[if");

Done.

[gagan.goku, 2010-10-04 13:28:08]

LGTM. Please send out to dev@.

http://codereview.appspot.com/2085042/diff/21001/java/gadgets/src/main/java/o...
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java
(right):

http://codereview.appspot.com/2085042/diff/21001/java/gadgets/src/main/java/o...
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java:314:
return commentText.startsWith("[if");
small nit:
return node.getNodeValue().trim().startsWith("[if");

[satya3656, 2010-10-04 13:33:21]

Addressing the comments.

[satya3656, 2010-10-04 13:33:58]

http://codereview.appspot.com/2085042/diff/21001/java/gadgets/src/main/java/o...
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java
(right):

http://codereview.appspot.com/2085042/diff/21001/java/gadgets/src/main/java/o...
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java:314:
return commentText.startsWith("[if");
On 2010/10/04 13:28:08, gagan.goku wrote:
> small nit:
> return node.getNodeValue().trim().startsWith("[if");

Done.

[Paul Lindner, 2010-10-04 15:35:02]

lgtm, if you could add a follow-on test for conditional comments I'd appreciate
it.

[gagan.goku, 2010-10-04 15:50:08]

http://codereview.appspot.com/2085042/diff/27001/java/gadgets/src/test/java/o...
nice tests for conditional and non conditional comments.

On Mon, Oct 4, 2010 at 9:05 PM, <lindner@inuus.com> wrote:

> lgtm, if you could add a follow-on test for conditional comments I'd
> appreciate it.
>
>
> http://codereview.appspot.com/2085042/
>

[Paul Lindner, 2010-10-04 16:09:39]

k thx, ship it!


On Mon, Oct 4, 2010 at 8:49 AM, Gagandeep singh <gagan.goku@gmail.com> wrote:
>
http://codereview.appspot.com/2085042/diff/27001/java/gadgets/src/test/java/o...
> has nice tests for conditional and non conditional comments.
>
> On Mon, Oct 4, 2010 at 9:05 PM, <lindner@inuus.com> wrote:
>>
>> lgtm, if you could add a follow-on test for conditional comments I'd
>> appreciate it.
>>
>> http://codereview.appspot.com/2085042/
>
>



-- 
Paul Lindner -- lindner@inuus.com -- linkedin.com/in/plindner

[satya3656, 2010-10-04 16:11:05]

Thanks Paul.

On 4 October 2010 21:39, Paul Lindner <lindner@inuus.com> wrote:

> k thx, ship it!
>
>
> On Mon, Oct 4, 2010 at 8:49 AM, Gagandeep singh <gagan.goku@gmail.com>
> wrote:
> >
>
http://codereview.appspot.com/2085042/diff/27001/java/gadgets/src/test/java/o...
> > has nice tests for conditional and non conditional comments.
> >
> > On Mon, Oct 4, 2010 at 9:05 PM, <lindner@inuus.com> wrote:
> >>
> >> lgtm, if you could add a follow-on test for conditional comments I'd
> >> appreciate it.
> >>
> >> http://codereview.appspot.com/2085042/
> >
> >
>
>
>
> --
> Paul Lindner -- lindner@inuus.com -- linkedin.com/in/plindner
>

[gagan.goku, 2010-10-04 17:23:09]

Build looks nice.
Committed as r1004328