-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Conversation
For HTML files, we replace the HTML sections with whitespace, leaving only the text of the script tags for Tern to see. This way the offset/cursor positions don't need to be translated, which would be necessary to get jump-to-def to work.
Also fix some bugs with jump-to-def and function type hints uncovered by the tests.
Session.prototype.getJavascriptText = function () { | ||
if (LanguageManager.getLanguageForPath(this.editor.document.file.fullPath).getId() === "html") { | ||
// HTML file - need to send back only the bodies of the | ||
// <script> tags |
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.
There's code very similar to this in HTMLUtils.findStyleBlocks(). Should we consolidate?
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.
@peterflynn good idea, will do
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.
refactored HTMLUtils.findStyleBlocks in eztierney@5d7dc76
The js code hinting code needed to find script blocks, so refactor the style block code to find any kind of block, and call that instead of having another copy of the code. findStyleBlocks now just calls the generic method, looking for "css" blocks. Also fixed up the block finding code to handle empty blocks, which I ran into when looking for "javascript" blocks.
Fixup branch so it will merge cleanly again. Conflicts: src/extensions/default/JavaScriptCodeHints/unittests.js
@@ -293,7 +312,7 @@ define(function (require, exports, module) { | |||
// type has changed since the last hint computation | |||
if (this.needNewHints(session)) { | |||
var offset = session.getOffset(), |
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.
Is "offset" used? Can you remove the variable declaration?
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.
I guess it is not, will delete.
I've reviewed the JS Code Hinting changes. Could someone in brackets review the HTMLUtils.js changes. Thanks. |
Sure. I'll review the HTMLUtils.js changes. |
To @RaymondLim for review |
currentBlock.end = currentBlock.start; | ||
} | ||
// Check for end of this block | ||
if (tokenModeName !== modeName) { | ||
// currentStyleBlock.end is already set to pos of the last CSS token by now |
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.
Need to update this comment (or just remove it) since it is referring to style block.
Done reviewing. I found only one minor nit for an old comment. |
Updated the old comment to no longer refer to style blocks, or CSS tokens |
[JS Code Hints] Html script hinting
merged. |
Implement codehints and jump-to-def for javascript inside <script> tags in a .html file.
To support html hinting, the code strips out the non-javascript parts of the html file. We replace them with whitespace so that the offset and cursor positions don't have to be translated. Then the javascript-only contents are sent to tern for processing so that the html tags don't confuse the js parser tern uses.
Also adds a few units tests to make sure html hinting continues to work.