url hinting support #1747
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,12 +29,17 @@ define(function (require, exports, module) { | |
| "use strict"; | ||
|
|
||
| // Load dependent modules | ||
| var CodeHintManager = brackets.getModule("editor/CodeHintManager"), | ||
| DocumentManager = brackets.getModule("document/DocumentManager"), | ||
| EditorManager = brackets.getModule("editor/EditorManager"), | ||
| HTMLUtils = brackets.getModule("language/HTMLUtils"), | ||
| NativeFileSystem = brackets.getModule("file/NativeFileSystem").NativeFileSystem, | ||
| ProjectManager = brackets.getModule("project/ProjectManager"), | ||
| StringUtils = brackets.getModule("utils/StringUtils"), | ||
| HTMLTags = require("text!HtmlTags.json"), | ||
| HTMLAttributes = require("text!HtmlAttributes.json"), | ||
| tags = JSON.parse(HTMLTags), | ||
| attributes = JSON.parse(HTMLAttributes); | ||
|
|
||
| /** | ||
| * @constructor | ||
|
|
@@ -88,8 +93,9 @@ define(function (require, exports, module) { | |
| * @param {string} completion - text to insert into current code editor | ||
| * @param {Editor} editor | ||
| * @param {Cursor} current cursor location | ||
| * @param {boolean} closeHints - true to close hints, or false to continue hinting | ||
| */ | ||
| TagHints.prototype.handleSelect = function (completion, editor, cursor, closeHints) { | ||
| var start = {line: -1, ch: -1}, | ||
| end = {line: -1, ch: -1}, | ||
| tagInfo = HTMLUtils.getTagInfo(editor, cursor), | ||
|
|
@@ -126,6 +132,7 @@ define(function (require, exports, module) { | |
| */ | ||
| function AttrHints() { | ||
| this.globalAttributes = this.readGlobalAttrHints(); | ||
| this.cachedHints = null; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -146,8 +153,9 @@ define(function (require, exports, module) { | |
| * @param {string} completion - text to insert into current code editor | ||
| * @param {Editor} editor | ||
| * @param {Cursor} current cursor location | ||
| * @param {boolean} closeHints - true to close hints, or false to continue hinting | ||
| */ | ||
| AttrHints.prototype.handleSelect = function (completion, editor, cursor, closeHints) { | ||
| var start = {line: -1, ch: -1}, | ||
| end = {line: -1, ch: -1}, | ||
| tagInfo = HTMLUtils.getTagInfo(editor, cursor), | ||
|
|
@@ -195,15 +203,17 @@ define(function (require, exports, module) { | |
| } | ||
| } | ||
|
|
||
| if (closeHints) { | ||
| if (insertedName) { | ||
| editor.setCursorPos(start.line, start.ch + completion.length - 1); | ||
|
|
||
| // Since we're now inside the double-quotes we just inserted, | ||
| // immediately pop up the attribute value hint. | ||
| CodeHintManager.showHint(editor); | ||
| } else if (tokenType === HTMLUtils.ATTR_VALUE && tagInfo.attr.hasEndQuote) { | ||
| // Move the cursor to the right of the existing end quote after value insertion. | ||
| editor.setCursorPos(start.line, start.ch + completion.length + 1); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
|
|
@@ -246,6 +256,144 @@ define(function (require, exports, module) { | |
| return query; | ||
| }; | ||
|
|
||
| /** | ||
| * Helper function for search(). Create a list of urls to existing files based on the query. | ||
| * @param {Object.<queryStr: string, ...} query -- a query object with a required property queryStr | ||
| * that will be used to filter out code hints | ||
| * @return {Array.<string>} | ||
| */ | ||
| AttrHints.prototype._getUrlList = function (query) { | ||
| var doc, | ||
| result = []; | ||
|
|
||
| // site-root relative links are not yet supported, so filter them out | ||
| if (query.queryStr.length > 0 && query.queryStr[0] === "/") { | ||
| return result; | ||
| } | ||
|
|
||
| // get path to current document | ||
| doc = DocumentManager.getCurrentDocument(); | ||
| if (!doc || !doc.file) { | ||
| return result; | ||
| } | ||
|
|
||
| var docUrl = window.PathUtils.parseUrl(doc.file.fullPath); | ||
| if (!docUrl) { | ||
| return result; | ||
| } | ||
|
|
||
| var docDir = docUrl.domain + docUrl.directory; | ||
|
|
||
| // get relative path from query string | ||
| // TODO: handle site-root relative | ||
| var queryDir = ""; | ||
| var queryUrl = window.PathUtils.parseUrl(query.queryStr); | ||
| if (queryUrl) { | ||
| queryDir = queryUrl.directory; | ||
| } | ||
|
|
||
| // build target folder path | ||
| var targetDir = docDir + decodeURI(queryDir); | ||
|
There was a problem hiding this comment. Why do you need decodeURI here if you're going to show encoded urls in the list? There was a problem hiding this comment. targetDir is the folder path passed to NativeFileSystem.requestNativeFileSystem() to retrieve the contents of that folder. It is not seen by user. The list that it returns is the list used for hints, which is encoded. |
||
|
|
||
| // get list of files from target folder | ||
| var unfiltered = []; | ||
|
|
||
| // Getting the file/folder info is an asynch operation, so it works like this: | ||
| // | ||
| // The initial pass initiates the asynchronous retrieval of data and returns an | ||
| // empty list, so no code hints are displayed. In the async callback, the code | ||
| // hints and the original query are stored in a cache, and then the process to | ||
| // show code hints is re-initiated. | ||
| // | ||
| // During the next pass, there should now be code hints cached from the initial | ||
| // pass, but user may have typed while file/folder info was being retrieved from | ||
| // disk, so we need to make sure code hints still apply to current query. If so, | ||
| // display them, otherwise, clear cache and start over. | ||
| // | ||
| // As user types within a folder, the same unfiltered file/folder list is still | ||
| // valid and re-used from cache. Filtering based on user input is done outside | ||
| // of this method. When user moves to a new folder, then the cache is deleted, | ||
| // and file/folder info for new folder is then retrieved. | ||
|
|
||
| if (this.cachedHints) { | ||
| // url hints have been cached, so determine if they're stale | ||
| if (!this.cachedHints.query || | ||
| this.cachedHints.query.tag !== query.tag || | ||
| this.cachedHints.query.attrName !== query.attrName || | ||
| this.cachedHints.queryDir !== queryDir) { | ||
|
|
||
| // delete stale cache | ||
| this.cachedHints = null; | ||
| } | ||
| } | ||
|
|
||
| if (this.cachedHints) { | ||
| // if (cachedHints.waiting === true), then we'll return an empty list | ||
| // and not start another readEntries() operation | ||
| unfiltered = this.cachedHints.unfiltered; | ||
|
|
||
| } else { | ||
| var self = this, | ||
| origEditor = EditorManager.getFocusedEditor(); | ||
|
|
||
| // create empty object so we can detect "waiting" state | ||
| self.cachedHints = {}; | ||
| self.cachedHints.unfiltered = []; | ||
| self.cachedHints.waiting = true; | ||
|
There was a problem hiding this comment. I don't see you checking on this boolean cachedHints.waiting. If you don't need it anymore, then you should remove all instances. There was a problem hiding this comment. Good catch! Fixed. I ran into a problem where the waiting flag got stuck on so cache was never cleared. Now, we just return the list whether it had items or not. |
||
|
|
||
| NativeFileSystem.requestNativeFileSystem(targetDir, function (dirEntry) { | ||
| dirEntry.createReader().readEntries(function (entries) { | ||
|
|
||
| entries.forEach(function (entry) { | ||
|
There was a problem hiding this comment. For consistency with other existing code, can you replace .forEach calls with $.map? There was a problem hiding this comment. I disagree with this suggestion. We are supposed to use Array.foreach instead of I think we may want to convert calls from $.map to Array.map in the places where the operation is on an Array (not a jQuery result set). If so, then it might make sense to convert my code to Array.map. There was a problem hiding this comment. Nice performance data! I agree with you. We can make the decision later on. |
||
| if (ProjectManager.shouldShow(entry)) { | ||
| // convert to doc relative path | ||
| var entryStr = entry.fullPath.replace(docDir, ""); | ||
|
|
||
| // code hints show the same strings that are inserted into text, | ||
| // so strings in list will be encoded. wysiwyg, baby! | ||
| unfiltered.push(encodeURI(entryStr)); | ||
|
There was a problem hiding this comment. encodeURI won't encode '?' and ';' that can be used in filenames. We may have to consider encodeURIComponent or a combination of escape and encodeURI. I can't remember what we did for our default brackets folder path. There was a problem hiding this comment. This is just encoding existing file and folder names retrieved from disk. So, there won't be any '?' or ';' chars. This is not user input. Maybe I do not understand your concern. There was a problem hiding this comment. If an existing filename or folder name has these characters, then we won't be encoding them after url insertion. There was a problem hiding this comment. This is the same encoding function we use every where in Brackets so I'm not sure why it's an issue for this feature. Since this is just a bonus feature, can we add this issue to the followup user story? There was a problem hiding this comment. I understand. I'm just trying to help you making this bonus feature better. I will try to find the potential issue with these characters later in Brackets. |
||
| } | ||
| }); | ||
|
|
||
| self.cachedHints.unfiltered = unfiltered; | ||
| self.cachedHints.query = query; | ||
| self.cachedHints.queryDir = queryDir; | ||
| self.cachedHints.waiting = false; | ||
|
|
||
| // If the editor has not changed, then re-initiate code hints. Cached data | ||
| // is still valid for folder even if we're not going to show it now. | ||
| if (origEditor === EditorManager.getFocusedEditor()) { | ||
| CodeHintManager.showHint(origEditor); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| return result; | ||
| } | ||
|
|
||
| // build list | ||
|
|
||
| // without these entries, typing "../" will not display entries for containing folder | ||
| if (queryUrl.filename === ".") { | ||
| result.push(queryDir + "."); | ||
| } else if (queryUrl.filename === "..") { | ||
| result.push(queryDir + ".."); | ||
| } | ||
|
|
||
| // add file/folder entries | ||
| unfiltered.forEach(function (item) { | ||
| result.push(item); | ||
| }); | ||
|
|
||
| // TODO: filter by desired file type based on tag, type attr, etc. | ||
|
|
||
| // TODO: add list item to top of list to popup modal File Finder dialog | ||
| // New string: "Browse..." or "Choose a File..." | ||
| // Command: Commands.FILE_OPEN | ||
|
|
||
| return result; | ||
| }; | ||
|
|
||
| /** | ||
| * Create a complete list of attributes for the tag in the query. Then filter | ||
| * the list by attrName in the query and return the result. | ||
|
|
@@ -260,7 +408,8 @@ define(function (require, exports, module) { | |
| var tagName = query.tag, | ||
| attrName = query.attrName, | ||
| filter = query.queryStr, | ||
| unfiltered = [], | ||
| sortFunc = null; | ||
|
|
||
| if (attrName) { | ||
| // We look up attribute values with tagName plus a slash and attrName first. | ||
|
|
@@ -274,6 +423,9 @@ define(function (require, exports, module) { | |
| if (attrInfo) { | ||
| if (attrInfo.type === "boolean") { | ||
| unfiltered = ["false", "true"]; | ||
| } else if (attrInfo.type === "url") { | ||
| unfiltered = this._getUrlList(query); | ||
|
There was a problem hiding this comment. I didn't see you encoding the path for unsafe characters like '(', ')', or space character in _getUrlList. If you're not going to show encoded urls in code hints, then at least you have to encode it before insertion. You also need to decode query before passing it to _getUrlList so that it can match to items in code hint list. There was a problem hiding this comment. Fixed. The encoded url is shown in the list and inserted in the page. There was a problem hiding this comment. Actually, I prefer the other solution. But I think it is acceptable to show encoded url in the list. There was a problem hiding this comment. I tried to implement what you described, but it's too hard to get filtering correct. Part of the reason is that it implies you can, for example, filter by typing a space char when %20 will get inserted in page, which is a conflict . I think it's best to show user encoded url and they learn how they should really look, so I think this is the best solution. There was a problem hiding this comment. Not quite if we have to consider double-byte characters and high-ascii characters. Anyway, we can leave it for users who can't stand to read encoded url to log an issue. |
||
| sortFunc = StringUtils.urlSort; | ||
| } else if (attrInfo.attribOption) { | ||
| unfiltered = attrInfo.attribOption; | ||
| } | ||
|
|
@@ -285,11 +437,12 @@ define(function (require, exports, module) { | |
| } | ||
|
|
||
| if (unfiltered.length) { | ||
| console.assert(!result.length); | ||
| result = $.map(unfiltered, function (item) { | ||
|
There was a problem hiding this comment. For folder and file sorting you need to call your custom sort() function here instead of $.map().sort(). There was a problem hiding this comment. Fixed. I pass a custom sorting function to sort(). |
||
| if (item.indexOf(filter) === 0) { | ||
| return item; | ||
| } | ||
| }).sort(sortFunc); | ||
| } | ||
| } | ||
|
|
||
|
|
||
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.
Since site relative path is not supported yet, you need to check whether query.Str starts with "/". If it is, then just return here so that we're not showing incorrect code hints.
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.
Good catch! Fixed.