Bug 313624

Comment 1 Ian Tewksbury 2010-05-19 16:39:25 EDT

Created attachment 169222 [details]
Enhancement Patch

It took a bit of finagling to get this to work, but mostly because the LineStyleProviderForJSDT is a bit of a mess.

I marked this as an enhancement because while I was fixing this issue I also got rid of some dead code in there and cleaned up the code path a bit. No I did not go to town fixing code style issues :).

Also while I was in there I fixed it so that single line and multi line style comments use the correct respective style preference, right now for script regions both types of comments use the single line comment style property.  I did not break this off in a separate bug because merging the two patches later seemed like a pain, if you are interested in just this fix for a sooner time frame I can break it out.

I won't spell out the code but the end result is that now <!-- and --> in JS regions follow syntax coloring preference set for HTML pages.  This even works when the users uses //--> for the closing tag where the // gets highlighted as JS comment as it should and the --> gets highlighted as an HTML comment as it should (that took inventing a new type of SingleLineRule which you can find in JSDTCodeScanner).

Not sure if there would be any good way to write a JUnit for this, but I am open to suggestions.  I also get stumped when it comes to testing graphical enhancements.

Comment 2 Chris Jaun 2010-05-19 17:19:13 EDT

I think this is a bit much for the RC build and a relatively minor problem.

Moving out to 3.2.1.

Comment 3 Ian Tewksbury 2010-09-28 14:51:35 EDT

Created attachment 179777 [details]
Fix Patch

I updated the patch so that it could handle text coming after an HTML style comment on the same line.  This issue was brought up by Bug 326303.

Example:

<script type="text/javascript">
<!--  THIS SHOULD BE SYNTAX HIGHLIGHTED AS HTML COMMENT TEXT
var v = "foo";
// -->
</script>

Comment 4 Ian Tewksbury 2010-09-29 09:32:22 EDT

It was brought to my attention that the style on the text after the HTML style comment would flicker on and off as you type.

The reason for this is Bug 326303.  The annotation processor is adding and removing the (invalid) error marker on that line and when it adds it back it marks only the text as dirty.  Then syntax coloring rule processor only processes the text and thus does not match on the comment rule.

Now I could update this patch to update the syntax rules to deal with this, but its not pretty.  Really I think the best solution is that the fix for Bug 326303 goes in fist and then this one, then there is not an issue.

Comment 5 Ian Tewksbury 2010-09-29 10:07:07 EDT

Created attachment 179848 [details]
Screen Shot Comparison of Options

This is a screen shot comparison of the three different options we have to fix this.

I prefer Option 2 though Nitin mentioned he maybe partial to Option 3.

Patches to follow.

Comment 6 Ian Tewksbury 2010-09-29 10:07:54 EDT

Created attachment 179849 [details]
Patch - Option 1

Patch corresponding to option 1 in the previously attached image.

Comment 7 Ian Tewksbury 2010-09-29 10:08:23 EDT

Created attachment 179850 [details]
Patch - Option 2

Patch corresponding to option 1 in the previously attached image.
This is my personal favorite.

Comment 8 Ian Tewksbury 2010-09-29 10:08:55 EDT

Created attachment 179851 [details]
Patch - Option 3

Patch corresponding to option 3 in the previously attached image.

Comment 9 Ian Tewksbury 2010-09-29 10:09:15 EDT

(In reply to comment #7)
> Created an attachment (id=179850) [details]
> Patch - Option 2
> 
> Patch corresponding to option 1 in the previously attached image.
> This is my personal favorite.

That should have said:
"Patch corresponding to option 2 in the previously attached image."

Comment 10 Chris Jaun 2010-10-01 17:06:18 EDT

Checked into 3.2.3 and HEAD.