Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Added Code to HtmlUtils to return info when in closing tag #3419

Merged
merged 5 commits into from
Apr 20, 2013
Merged

Added Code to HtmlUtils to return info when in closing tag #3419

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
391342c
Added code to return tagName when in closing tag
WebsiteDeveloper Apr 11, 2013
a079da6
A few refactorings
WebsiteDeveloper Apr 11, 2013
b507ad0
Fixes after first review
WebsiteDeveloper Apr 13, 2013
f50e376
Fixes after second review
WebsiteDeveloper Apr 16, 2013
53cbe28
Minor fixes after review
WebsiteDeveloper Apr 20, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/editor/CSSInlineEditor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ define(function (require, exports, module) {
var tagInfo = HTMLUtils.getTagInfo(editor, pos),
selectorName = "";

if (tagInfo.position.tokenType === HTMLUtils.TAG_NAME) {
if (tagInfo.position.tokenType === HTMLUtils.TAG_NAME || tagInfo.position.tokenType === HTMLUtils.CLOSING_TAG) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to add a new unit test for this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You still need to add a unit test for Quick Edit in closing tag to test this specific change, probably in InlineEditorProviders-test.js.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems i've missed that

// Type selector
selectorName = tagInfo.tagName;
} else if (tagInfo.position.tokenType === HTMLUtils.ATTR_NAME ||
Expand Down
22 changes: 14 additions & 8 deletions src/language/HTMLUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ define(function (require, exports, module) {

//constants
var TAG_NAME = "tagName",
CLOSING_TAG = "closingTag",
ATTR_NAME = "attr.name",
ATTR_VALUE = "attr.value";

Expand Down Expand Up @@ -399,12 +400,16 @@ define(function (require, exports, module) {
return createTagInfo();
}

// Check to see if this is the closing of a tag (either the start or end)
if (ctx.token.string === ">" || ctx.token.string === "/>" ||
(ctx.token.string.charAt(0) === "<" && ctx.token.string.charAt(1) === "/")) {
// Check to see if this is the closing of a tag (either the start or end) self closing tag
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also update this comment to " the closing of a start tag or self closing tag."

if (ctx.token.string === ">" || ctx.token.string === "/>") {
return createTagInfo();
}

// Check to see if this is the closing of a tag (either the start or end) closing tag
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (ctx.token.string.charAt(0) === "<" && ctx.token.string.charAt(1) === "/") {
return createTagInfo(CLOSING_TAG, offset + 2, ctx.token.string.slice(2));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually i misinterpreted the meaning of the offset, now it is clear to me.

}

// Make sure the cursor is not after an equal sign or a quote before we report the context as a tag.
if (ctx.token.string !== "=" && ctx.token.string.match(/^["']/) === null) {
if (!tokenType) {
Expand Down Expand Up @@ -504,14 +509,15 @@ define(function (require, exports, module) {


// Define public API
exports.TAG_NAME = TAG_NAME;
exports.ATTR_NAME = ATTR_NAME;
exports.ATTR_VALUE = ATTR_VALUE;
exports.TAG_NAME = TAG_NAME;
exports.CLOSING_TAG = CLOSING_TAG;
exports.ATTR_NAME = ATTR_NAME;
exports.ATTR_VALUE = ATTR_VALUE;

exports.getTagInfo = getTagInfo;
exports.getTagInfo = getTagInfo;
exports.getTagAttributes = getTagAttributes;
//The createTagInfo is really only for the unit tests so they can make the same structure to
//compare results with
exports.createTagInfo = createTagInfo;
exports.createTagInfo = createTagInfo;
exports.findStyleBlocks = findStyleBlocks;
});
20 changes: 10 additions & 10 deletions test/spec/CodeHintUtils-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,16 @@ define(function (require, exports, module) {
expect(tag).toEqual(HTMLUtils.createTagInfo(HTMLUtils.TAG_NAME));
});

it("should hint tagname for a closing tag", function () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're not providing tag hinting in closing tags by your changes. Your changes in this pull request just change the behavior in Quick Edit, not other features. So you need to keep the original test title.

var pos = {"ch": 0, "line": 0};
setContentAndUpdatePos(pos,
["<html>", "<body>", "<div id='test' class='foo'></div>"],
"</body></ht", "ml>");

var tag = HTMLUtils.getTagInfo(myEditor, pos);
expect(tag).toEqual(HTMLUtils.createTagInfo(HTMLUtils.CLOSING_TAG, 6, "html"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, offset is wrong here. See the comment above.

});

it("should find the tagname of the current tag if two tags are right next to each other", function () {
var pos = {"ch": 0, "line": 0};
setContentAndUpdatePos(pos,
Expand Down Expand Up @@ -237,16 +247,6 @@ define(function (require, exports, module) {
expect(tag).toEqual(HTMLUtils.createTagInfo());
});

it("should not hint anything inside a closing tag", function () {
var pos = {"ch": 0, "line": 0};
setContentAndUpdatePos(pos,
["<html>", "<body>", "<div id='test' class='foo'></div>"],
"</body></ht", "ml>");

var tag = HTMLUtils.getTagInfo(myEditor, pos);
expect(tag).toEqual(HTMLUtils.createTagInfo());
});

it("should not find attributes in an empty editor", function () {
var pos = {"ch": 0, "line": 0};
var attrs = HTMLUtils.getTagAttributes(myEditor, pos);
Expand Down