Added Code to HtmlUtils to return info when in closing tag #3419
Conversation
|
Definitely needs unit tests |
ghost
assigned 
| return createTagInfo(); | ||
| // Check to see if this is the closing of a tag (either the start or end) self closing tag | ||
| if (ctx.token.string === ">" || ctx.token.string === "/>") { | ||
| return createTagInfo(CLOSING_TAG, offset); |
There was a problem hiding this comment.
I don't think we need this unless there is a specific use of "CLOSING_TAG" in any feature. Otherwise, you may introduce some unit test failures here. So I suggest that you keep the empty tag info.
|
@RaymondLim changes pushed. |
| return createTagInfo(); | ||
| } | ||
|
|
||
| // Check to see if this is the closing of a tag (either the start or end) closing tag | ||
| if (ctx.token.string.charAt(0) === "<" && ctx.token.string.charAt(1) === "/") { | ||
| return createTagInfo(CLOSING_TAG, offset + 2, ctx.token.string.slice(2)); |
There was a problem hiding this comment.
You're adjusting the offset in the wrong direction. Consider this example: </|h2> where | is the cursor position. You will have offset equals to 2 and token string equals to "</h2" before your changes. Since you're stripping off "</" from the token string, the caller will get "h2" as the tag name and you should have offset set to zero, indicating the cursor is before the returned string.
There was a problem hiding this comment.
actually i misinterpreted the meaning of the offset, now it is clear to me.
|
Done second review. |
|
@RaymondLim fixes pushed. |
| return createTagInfo(); | ||
| } | ||
|
|
||
| // Check to see if this is the closing of a tag (either the start or end) closing tag |
There was a problem hiding this comment.
Sorry, I should have caught this in the previous round of review. This comment is no longer correct. We're just handling closing tag here.
|
@RaymondLim fixes pushed. |
|
Looks good. Merging. |
Added Code to HtmlUtils to return info when in closing tag
Possible fix for #3407 and #1570.
@RaymondLim you may want to look at this.