From 4be2a8aecefbd6b77c74af95b834a8072e6dfde6 Mon Sep 17 00:00:00 2001 From: Peter Flynn Date: Fri, 28 Mar 2014 17:37:18 -0700 Subject: [PATCH 1/3] - Tweak Quick Edit error/feedback strings - Fix comments to reflect Quick Edit error related API changes - Fix outdated FileSyncManager docs --- src/editor/CSSInlineEditor.js | 2 +- src/editor/EditorManager.js | 16 ++++++++-------- .../default/JavaScriptQuickEdit/main.js | 4 ++-- src/nls/root/strings.js | 10 +++++----- src/project/FileSyncManager.js | 9 ++++++--- 5 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/editor/CSSInlineEditor.js b/src/editor/CSSInlineEditor.js index bc8411da2fb..2d2fd768999 100644 --- a/src/editor/CSSInlineEditor.js +++ b/src/editor/CSSInlineEditor.js @@ -162,7 +162,7 @@ define(function (require, exports, module) { * @param {!{line:Number, ch:Number}} pos * @return {?$.Promise} synchronously resolved with an InlineWidget, or * {string} if pos is in tag but not in tag name, class attr, or id attr, or - * null if we're not going to provide anything. + * null if we're not even close to a context where we could provide anything. */ function htmlToCSSProvider(hostEditor, pos) { diff --git a/src/editor/EditorManager.js b/src/editor/EditorManager.js index 4723e28dcac..b47d1b32ddc 100644 --- a/src/editor/EditorManager.js +++ b/src/editor/EditorManager.js @@ -164,7 +164,7 @@ define(function (require, exports, module) { * @param {!Editor} editor The host editor * @param {Array.<{priority:number, provider:function(...)}>} providers * prioritized list of providers - * @param {string=} defaultErrorMsg Default error message to display if no initial provider found + * @param {string=} defaultErrorMsg Default message to display if no providers return non-null * @return {$.Promise} a promise that will be resolved when an InlineWidget * is created or rejected if no inline providers have offered one. */ @@ -279,10 +279,10 @@ define(function (require, exports, module) { * An optional priority parameter is used to give providers with higher priority an opportunity * to provide an inline editor before providers with lower priority. * - * @param {function(!Editor, !{line:number, ch:number}):?$.Promise} provider + * @param {function(!Editor, !{line:number, ch:number}):?($.Promise|string)} provider * @param {number=} priority - * The provider returns a promise that will be resolved with an InlineWidget, or returns null - * to indicate the provider doesn't want to respond to this case. + * The provider returns a promise that will be resolved with an InlineWidget, or returns a string + * indicating why the provider cannot respond to this case (or returns null to indicate no reason). */ function registerInlineEditProvider(provider, priority) { if (priority === undefined) { @@ -297,10 +297,10 @@ define(function (require, exports, module) { * An optional priority parameter is used to give providers with higher priority an opportunity * to provide an inline editor before providers with lower priority. * - * @param {function(!Editor, !{line:number, ch:number}):?$.Promise} provider + * @param {function(!Editor, !{line:number, ch:number}):?($.Promise|string)} provider * @param {number=} priority - * The provider returns a promise that will be resolved with an InlineWidget, or returns null - * to indicate the provider doesn't want to respond to this case. + * The provider returns a promise that will be resolved with an InlineWidget, or returns a string + * indicating why the provider cannot respond to this case (or returns null to indicate no reason). */ function registerInlineDocsProvider(provider, priority) { if (priority === undefined) { @@ -946,7 +946,7 @@ define(function (require, exports, module) { * * @param {Array.<{priority:number, provider:function(...)}>} providers * prioritized list of providers - * @param {string=} errorMsg Error message to display if no initial provider found + * @param {string=} errorMsg Default message to display if no providers return non-null * @return {!Promise} A promise resolved with true if an inline widget is opened or false * when closed. Rejected if there is neither an existing widget to close nor a provider * willing to create a widget (or if no editor is open). diff --git a/src/extensions/default/JavaScriptQuickEdit/main.js b/src/extensions/default/JavaScriptQuickEdit/main.js index f93bd45e0dd..5b39eae9c0b 100644 --- a/src/extensions/default/JavaScriptQuickEdit/main.js +++ b/src/extensions/default/JavaScriptQuickEdit/main.js @@ -42,7 +42,7 @@ define(function (require, exports, module) { * * @param hostEditor {!Editor} editor * @param {!{line:Number, ch:Number}} pos - * @return {functionName: {string}, reason: {string}} + * @return {functionName: string, reason: string} */ function _getFunctionName(hostEditor, pos) { var token = hostEditor._codeMirror.getTokenAt(pos, true); @@ -109,7 +109,7 @@ define(function (require, exports, module) { * @param {!string} functionName * @return {?$.Promise} synchronously resolved with an InlineWidget, or * {string} if js other than function is detected at pos, or - * null if we're not going to provide anything. + * null if we're not ready to provide anything. */ function _createInlineEditor(hostEditor, functionName) { // Use Tern jump-to-definition helper, if it's available, to find InlineEditor target. diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js index 8e76da3f0ac..d9f2281ae11 100644 --- a/src/nls/root/strings.js +++ b/src/nls/root/strings.js @@ -180,15 +180,15 @@ define({ "FILE_FILTER_CLIPPED_SUFFIX" : "and {0} more", // Quick Edit - "ERROR_QUICK_EDIT_PROVIDER_NOT_FOUND" : "No Quick Edit provider found for current cursor position", - "ERROR_CSSQUICKEDIT_CLASSNOTFOUND" : "CSS Quick Edit: place cursor in class name", - "ERROR_CSSQUICKEDIT_IDNOTFOUND" : "CSS Quick Edit: place cursor in id name", - "ERROR_CSSQUICKEDIT_UNSUPPORTEDATTR" : "CSS Quick Edit: place cursor in tag name, class name, or id name", + "ERROR_QUICK_EDIT_PROVIDER_NOT_FOUND" : "No Quick Edit available for current cursor position", + "ERROR_CSSQUICKEDIT_CLASSNOTFOUND" : "CSS Quick Edit: incomplete class attribute", + "ERROR_CSSQUICKEDIT_IDNOTFOUND" : "CSS Quick Edit: incomplete id attribute", + "ERROR_CSSQUICKEDIT_UNSUPPORTEDATTR" : "CSS Quick Edit: place cursor in tag name, class attribute, or id attribute", "ERROR_TIMINGQUICKEDIT_INVALIDSYNTAX" : "CSS Timing Function Quick Edit: invalid syntax", "ERROR_JSQUICKEDIT_FUNCTIONNOTFOUND" : "JS Quick Edit: place cursor in function name", // Quick Docs - "ERROR_QUICK_DOCS_PROVIDER_NOT_FOUND" : "No Quick Docs provider found for current cursor position", + "ERROR_QUICK_DOCS_PROVIDER_NOT_FOUND" : "No Quick Docs available for current cursor position", /** * ProjectManager diff --git a/src/project/FileSyncManager.js b/src/project/FileSyncManager.js index cb60ec412d1..6a29fcd3e80 100644 --- a/src/project/FileSyncManager.js +++ b/src/project/FileSyncManager.js @@ -29,9 +29,12 @@ * FileSyncManager is a set of utilities to help track external modifications to the files and folders * in the currently open project. * - * Currently, we look for external changes purely by checking file timestamps against the last-sync - * timestamp recorded on Document. Later, we will use actual native directory-watching callbacks - * instead. + * Currently, we detect external changes purely by checking file timestamps against the last-sync + * timestamp recorded on Document. Brackets triggers this check whenever an external change was detected + * by our native file watchers, and on window focus. We recheck all open Documents, but with file caching + * the timestamp check is a fast no-op for everything other than files where a watcher change was just + * notified. If watchers/caching are disabled, we'll essentially check only on window focus, and we'll hit + * the disk to check every open Document's timestamp every time. * * FUTURE: Whenever we have a 'project file tree model,' we should manipulate that instead of notifying * DocumentManager directly. DocumentManager, the tree UI, etc. then all listen to that model for changes. From 6beff76dd563278cd2895722d401528977411fa8 Mon Sep 17 00:00:00 2001 From: Peter Flynn Date: Wed, 2 Apr 2014 14:22:50 -0700 Subject: [PATCH 2/3] Further improve Quck Edit error strings. Break out a new separate case for cursor pos being ambiguous between two class names. Fix some more JSDoc typos found in code review. --- src/editor/CSSInlineEditor.js | 30 +++++++++---------- .../default/JavaScriptQuickEdit/main.js | 4 +-- src/nls/root/strings.js | 3 +- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/editor/CSSInlineEditor.js b/src/editor/CSSInlineEditor.js index 2d2fd768999..ccdba6ce5a2 100644 --- a/src/editor/CSSInlineEditor.js +++ b/src/editor/CSSInlineEditor.js @@ -75,21 +75,21 @@ define(function (require, exports, module) { // class="error-dialog modal hide" // and the insertion point is inside "modal", we want ".modal" var attributeValue = tagInfo.attr.value; - var startIndex = attributeValue.substr(0, tagInfo.position.offset).lastIndexOf(" "); - var endIndex = attributeValue.indexOf(" ", tagInfo.position.offset); - selectorName = "." + - attributeValue.substring( - startIndex === -1 ? 0 : startIndex + 1, - endIndex === -1 ? attributeValue.length : endIndex - ); - - // If the insertion point is surrounded by space, selectorName is "." - // Check for that here - if (selectorName === ".") { - selectorName = ""; - } - - if (selectorName === "") { + if (attributeValue.trim()) { + var startIndex = attributeValue.substr(0, tagInfo.position.offset).lastIndexOf(" "); + var endIndex = attributeValue.indexOf(" ", tagInfo.position.offset); + selectorName = "." + + attributeValue.substring( + startIndex === -1 ? 0 : startIndex + 1, + endIndex === -1 ? attributeValue.length : endIndex + ); + + // If the insertion point is surrounded by space between two classnames, selectorName is "." + if (selectorName === ".") { + selectorName = ""; + reason = Strings.ERROR_CSSQUICKEDIT_BETWEENCLASSES; + } + } else { reason = Strings.ERROR_CSSQUICKEDIT_CLASSNOTFOUND; } } else if (tagInfo.attr.name === "id") { diff --git a/src/extensions/default/JavaScriptQuickEdit/main.js b/src/extensions/default/JavaScriptQuickEdit/main.js index 5b39eae9c0b..9e0f7f84e38 100644 --- a/src/extensions/default/JavaScriptQuickEdit/main.js +++ b/src/extensions/default/JavaScriptQuickEdit/main.js @@ -41,7 +41,7 @@ define(function (require, exports, module) { * Return the token string that is at the specified position. * * @param hostEditor {!Editor} editor - * @param {!{line:Number, ch:Number}} pos + * @param {!{line:number, ch:number}} pos * @return {functionName: string, reason: string} */ function _getFunctionName(hostEditor, pos) { @@ -186,7 +186,7 @@ define(function (require, exports, module) { * and shows (one/all of them) in an inline editor. * * @param {!Editor} editor - * @param {!{line:Number, ch:Number}} pos + * @param {!{line:number, ch:number}} pos * @return {$.Promise} a promise that will be resolved with an InlineWidget * or null if we're not going to provide anything. */ diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js index d9f2281ae11..7c19bff8b81 100644 --- a/src/nls/root/strings.js +++ b/src/nls/root/strings.js @@ -181,9 +181,10 @@ define({ // Quick Edit "ERROR_QUICK_EDIT_PROVIDER_NOT_FOUND" : "No Quick Edit available for current cursor position", + "ERROR_CSSQUICKEDIT_BETWEENCLASSES" : "CSS Quick Edit: place cursor on a single class name", "ERROR_CSSQUICKEDIT_CLASSNOTFOUND" : "CSS Quick Edit: incomplete class attribute", "ERROR_CSSQUICKEDIT_IDNOTFOUND" : "CSS Quick Edit: incomplete id attribute", - "ERROR_CSSQUICKEDIT_UNSUPPORTEDATTR" : "CSS Quick Edit: place cursor in tag name, class attribute, or id attribute", + "ERROR_CSSQUICKEDIT_UNSUPPORTEDATTR" : "CSS Quick Edit: place cursor in tag, class, or id", "ERROR_TIMINGQUICKEDIT_INVALIDSYNTAX" : "CSS Timing Function Quick Edit: invalid syntax", "ERROR_JSQUICKEDIT_FUNCTIONNOTFOUND" : "JS Quick Edit: place cursor in function name", From 136ec47cc0969368e95f90353f8d79d3b52e60af Mon Sep 17 00:00:00 2001 From: Peter Flynn Date: Thu, 3 Apr 2014 13:48:52 -0700 Subject: [PATCH 3/3] Code review: tweak wording of comment --- src/editor/CSSInlineEditor.js | 6 +++--- src/extensions/default/JavaScriptQuickEdit/main.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/editor/CSSInlineEditor.js b/src/editor/CSSInlineEditor.js index ccdba6ce5a2..e1548aa1e73 100644 --- a/src/editor/CSSInlineEditor.js +++ b/src/editor/CSSInlineEditor.js @@ -160,9 +160,9 @@ define(function (require, exports, module) { * * @param {!Editor} editor * @param {!{line:Number, ch:Number}} pos - * @return {?$.Promise} synchronously resolved with an InlineWidget, or - * {string} if pos is in tag but not in tag name, class attr, or id attr, or - * null if we're not even close to a context where we could provide anything. + * @return {?$.Promise} synchronously resolved with an InlineWidget; or error + * {string} if pos is in tag but not in tag name, class attr, or id attr; or null if the + * selection isn't even close to a context where we could provide anything. */ function htmlToCSSProvider(hostEditor, pos) { diff --git a/src/extensions/default/JavaScriptQuickEdit/main.js b/src/extensions/default/JavaScriptQuickEdit/main.js index 9e0f7f84e38..5b0303ea08f 100644 --- a/src/extensions/default/JavaScriptQuickEdit/main.js +++ b/src/extensions/default/JavaScriptQuickEdit/main.js @@ -188,7 +188,7 @@ define(function (require, exports, module) { * @param {!Editor} editor * @param {!{line:number, ch:number}} pos * @return {$.Promise} a promise that will be resolved with an InlineWidget - * or null if we're not going to provide anything. + * or null if we're not ready to provide anything. */ function javaScriptFunctionProvider(hostEditor, pos) { // Only provide a JavaScript editor when cursor is in JavaScript content