-
Notifications
You must be signed in to change notification settings - Fork 7.6k
[#10166] Highlight 'text/ng-template' (Angular) in script tags as HTML #10666
Conversation
|
|
I have just signed the brackets cla. Regarding your question, I was trying to follow the convention for the other mime types in the regex which did not include the "-template". However, on further thought, since "ng" is somewhat generic, it would probably be better to specify "ng-template" instead. I will change it soon. Thanks. |
|
@munxtwo Please make the required changes suggested by @peterflynn so that we can merge this to master. |
|
@nethip I already made the changes 4 days ago. are they not there? |
|
@munxtwo okay I was thinking the change was from ng-template to ng. So the correct thing is the other way round. Could you share any unit testing you have done around this change? Thanks |
|
@nethip i did not write any unit tests for this. I was not able to find any that were testing specifically around this. If you could point me to an example that would be great. Thanks. |
|
@munxtwo If you can just share what unit testing you have done that should be fine too. Having a written unit test is surely good to have, as other people can just run tests whenever some changes go in this area. By the way our test suite is present at the following location https://github.com/adobe/brackets/tree/master/test/spec/ You can look at the JS files at this location to get an understanding of how to write tests in Brackets. |
|
@nethip oh were you referring to the manual test cases that I have run? I basically used the code snippet that was provided in the issue description. Here are the test cases I did:
|
|
@munxtwo Thanks, this looks good. As you mentioned, we don't have any existing unit tests for this nested mode selection (it mostly leans on CodeMirror code anyway), so I don't think we need any for this PR. |
[#10166] Highlight 'text/ng-template' (Angular) in script tags as plain HTML
src/language/LanguageManager.js
Outdated
| CodeMirror.defineMIME("text/x-brackets-html", { | ||
| "name": "htmlmixed", | ||
| "scriptTypes": [{"matches": /\/x-handlebars|\/x-mustache|^text\/html$/i, | ||
| "scriptTypes": [{"matches": /\/x-handlebars|\/x-mustache|\/ng-|^text\/html$/i, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and x-template for vue.js?
No description provided.