github-linguist / linguist

Language Savant. If your repository's language is being reported incorrectly, send us a pull request!
MIT License
12.32k stars 4.26k forks source link

Server-side comments in ASPX (C# and VB) not highlighted as comments #895

Closed abitgone closed 9 years ago

abitgone commented 10 years ago

ASP.NET web forms – .aspx files, etc – can use server-side comments that are similar in syntax to HTML comments:

<%-- This is a server-side comment --%>

Linguist doesn't appear to be picking these up correctly.

arfon commented 9 years ago

Can you link to an example of this issue happening on GitHub?

abitgone commented 9 years ago
<%-- Sure, here you go --%>

I'd expect this to look the same as:

// This

/* or this */

Or even

<!-- This -->
arfon commented 9 years ago

Yes. Could you link to a file where you see this issue too please?

abitgone commented 9 years ago

I don't have any public repositories where this is the case. Give me a few minutes while I spin up a small repo that has a single .aspx file in it.

abitgone commented 9 years ago

Here you go: https://github.com/abitgone/linguist-aspx-comments/blob/master/test.aspx

pchaigno commented 9 years ago

The highlighting of this file seems better if you use text.html.asp instead of source.asp as we currently do:

It doesn't solve the bug with server-side comments but maybe we should start by changing that and open an issue at https://github.com/textmate/asp.tmbundle/...?

arfon commented 9 years ago

@abitgone - what are your thoughts on @pchaigno's last comment. This is an issue with the highlighting grammar https://github.com/textmate/asp.tmbundle/ - could you open an issue here to report the bug?

abitgone commented 9 years ago

@arfon: Couldn't agree more. I've submitted a pull request with a patch for the .tmbundle – you can see that my version highlights the server-side comments correctly now.

Thanks for your help.

infininight commented 9 years ago

@arfon I've accepted the pull request from @abitgone, ready to be pulled upstream.

arfon commented 9 years ago

@arfon I've accepted the pull request from @abitgone, ready to be pulled upstream.

Great! We should be updating the Linguist gem tomorrow.