-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Use language id instead of CodeMirror mode in CodeHintManager #3270
Conversation
@iwehrman might want to take a quick look at the changes to the JS hinting code, too. |
Looks OK to me, assuming "javascript" actually is the language ID for JavaScript. When testing this, it would be good idea to double-check that JavaScript hinting remains disabled on HTML pages inside of script tags. |
@iwehrman i double-checked it and it remains disabled. |
Good timing! I just sorted this in under "Straightforward refactoring" on the language support Wiki page. |
} | ||
} | ||
} | ||
|
||
if (registerInAllModes) { | ||
if (registerForAllLanguages) { | ||
// if we're registering in all, then we ignore the modeNames array |
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.
Missed one ("modeNames") :)
@DennisKehrig You can remove the JSLintUtils from that list now :) |
@WebsiteDeveloper Awesome work, looking forward to merging this! |
@TomMalbran Good point! I marked it as done, even though you did all the work. :) |
@DennisKehrig That is ok, just wanted to mention it. There is still a second bullet point about JSLint, that I think I covered in the refactor. |
@TomMalbran Thanks again :) I need sleep... |
@DennisKehrig all Done. |
Awesome, I'll check again on Monday! Thank you for your patience :) |
Use language id instead of CodeMirror mode in CodeHintManager
Merged! Seems to have worked well :) |
@WebsiteDeveloper @DennisKehrig this is a breaking API change for other CodeHintManager clients (e.g. the TypeScript extension). I've added this to the release notes, but can one of you place post a heads-up message in the forum also? |
@peterflynn i put up a notification in the google group |
Thanks! |
Odd, I installed all the extensions we collected for the extension manager extension, and some added later. The TypeScript extension slipped the feed somehow. Hope it was the only one affected by the change. |
A possible fix for: #3085.