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

Fix for Issue 3339 #3340

Merged
merged 7 commits into from
Apr 9, 2013
Merged

Fix for Issue 3339 #3340

Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
990fe39
Logic fix
WebsiteDeveloper Apr 4, 2013
edab6b8
A few other cleanup fixes
WebsiteDeveloper Apr 4, 2013
5ae84c4
Fixed replacement when currently in Entity
WebsiteDeveloper Apr 5, 2013
087fa43
A bit cleanup
WebsiteDeveloper Apr 8, 2013
b200259
Fix after review
WebsiteDeveloper Apr 8, 2013
cd9004a
Fixes after first review
WebsiteDeveloper Apr 8, 2013
e63ec5f
Fixes after second review
WebsiteDeveloper Apr 9, 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
68 changes: 38 additions & 30 deletions src/extensions/default/HtmlEntityCodeHints/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ define(function (require, exports, module) {
* The encoded string
*/
function _encodeValue(value) {
return (value.indexOf("#") === -1) ? value.replace("&", "&") : value.replace("&", "&").replace("#", "#");
return value.replace("&", "&").replace("#", "#");
}

/**
Expand All @@ -59,7 +59,7 @@ define(function (require, exports, module) {
* The decoded string
*/
function _decodeValue(value) {
return value.replace("#", "#").replace("&", "&").replace("&#59;", ";");
return value.replace("&", "&").replace("#", "#");
}

/**
Expand Down Expand Up @@ -92,11 +92,7 @@ define(function (require, exports, module) {

var query = this._getQuery();

if (implicitChar === null) {
return query !== null;
}

return implicitChar === "&" || query !== null;
return query !== null;
Copy link
Contributor

Choose a reason for hiding this comment

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

This method now returns query !== null in all cases, so remove this block:

    if (implicitChar === null) {
        return query !== null;
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, but now that you made that change, I can see that you don't need query var, and this can simply be:

        return this._getQuery() !== null;

Copy link
Contributor

Choose a reason for hiding this comment

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

Original comment seemed to be buried in a hidden "Outdated Diff", so adding again. Sorry for the dupe. Don't need query var, so this can be simplified more:

        return this._getQuery() !== null;

};

/**
Expand All @@ -120,16 +116,19 @@ define(function (require, exports, module) {
var query,
result;

if (this.primaryTriggerKeys.indexOf(implicitChar) !== -1 || implicitChar === null) {
if (implicitChar === null || this.primaryTriggerKeys.indexOf(implicitChar) !== -1) {
this.currentQuery = query = this._getQuery();
result = $.map(specialChars, function (value, index) {
if (value.indexOf(query) === 0) {
var shownValue = _encodeValue(value);
return shownValue + "&#59; <span class='entity-display-character'>" + value + ";</span>";
return shownValue + "; <span class='entity-display-character'>" + value + ";</span>";
}
}).sort(this._internalSort);

query = _encodeValue(query);
if (query !== null) {
query = _encodeValue(query);
}

return {
hints: result,
match: query,
Expand Down Expand Up @@ -171,32 +170,34 @@ define(function (require, exports, module) {
*/
SpecialCharHints.prototype._getQuery = function () {
var query,
lineContent,
lineContentBeforeCursor,
startChar,
endChar;
endChar,
cursor = this.editor.getCursorPos();

query = "&";
if (HTMLUtils.getTagInfo(this.editor, cursor).tagName !== "") {
return null;
Copy link
Contributor

Choose a reason for hiding this comment

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

The variable name lineContent should be more accurate so code is easier to understand, so rename to something like lineContentBeforeCursor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

}

lineContent = this.editor.document.getRange({
line: this.editor.getCursorPos().line,
lineContentBeforeCursor = this.editor.document.getRange({
line: cursor.line,
ch: 0
}, this.editor.getCursorPos());
}, cursor);

startChar = lineContent.lastIndexOf("&");
endChar = lineContent.lastIndexOf(";");
startChar = lineContentBeforeCursor.lastIndexOf("&");
endChar = lineContentBeforeCursor.lastIndexOf(";");

if (endChar < startChar) {
query = this.editor.document.getRange({
line: this.editor.getCursorPos().line,
ch: startChar
}, this.editor.getCursorPos());
}

if (startChar !== -1 && HTMLUtils.getTagInfo(this.editor, this.editor.getCursorPos()).tagName === "") {
return query;
// If no startChar was found or the endChar is greater than the startChar then it is no entity
if (startChar === -1 || endChar > startChar) {
return null;
}

return null;

query = this.editor.document.getRange({
line: cursor.line,
ch: startChar
}, cursor);

return query;
};

/**
Expand All @@ -212,11 +213,18 @@ define(function (require, exports, module) {
SpecialCharHints.prototype.insertHint = function (completion) {
var start = {line: -1, ch: -1},
end = {line: -1, ch: -1},
cursor = this.editor.getCursorPos();
cursor = this.editor.getCursorPos(),
match;

end.line = start.line = cursor.line;
start.ch = cursor.ch - this.currentQuery.length;
match = this.editor.document.getLine(cursor.line).slice(cursor.ch);
end.ch = start.ch + this.currentQuery.length;

if (match.indexOf(";") !== -1 && /^(#*[0-9]+)|([a-zA-Z]+)$/.test(match.slice(0, match.indexOf(";")))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

match.indexOf(";") is called twice, so it should be put in a var.

end.ch = this.editor.document.getLine(cursor.line).indexOf(";", start.ch) + 1;
}

completion = completion.slice(0, completion.indexOf(" "));
completion = _decodeValue(completion);
if (start.ch !== end.ch) {
Expand Down