Merge pull request #5612 from adobe/jbalsas/quickopen_cols

TomMalbran · TomMalbran · commit 734c236cd091 · 2014-01-18T22:47:44.000-08:00
Syntax for line:col jumps in QuickOpen search queries
diff --git a/src/search/QuickOpen.js b/src/search/QuickOpen.js
@@ -53,7 +53,10 @@ define(function (require, exports, module) {
         StringMatch         = require("utils/StringMatch"),
         ViewUtils           = require("utils/ViewUtils");
     
-
+    
+    /** @const {RegExp} The regular expression to check the cursor position */
+    var CURSOR_POS_EXP = new RegExp(":([^,]+)?(,(.+)?)?");
+    
     /** @type Array.<QuickOpenPlugin> */
     var plugins = [];
 
@@ -251,23 +254,28 @@ define(function (require, exports, module) {
      * is followed by a colon. Callers should explicitly test result with isNaN()
      * 
      * @param {string} query string to extract line number from
-     * @returns {number} line number. Returns NaN to indicate no line number was found
+     * @return {{query: string, local: boolean, line: number, ch: number}} An object with
+     *      the extracted line and column numbers, and two additional fields: query with the original position 
+     *      string and local indicating if the cursor position should be applied to the current file.
+     *      Or null if the query is invalid
      */
-    function extractLineNumber(query) {
-        // only match : at beginning of query for now
-        // TODO: match any location of : when QuickOpen._handleItemFocus() is modified to
-        // dynamic open files
-        if (query.indexOf(":") !== 0) {
-            return NaN;
-        }
-
-        var result = NaN;
-        var regInfo = query.match(/(!?:)(\d+)/); // colon followed by a digit
-        if (regInfo) {
-            result = regInfo[2] - 1;
+    function extractCursorPos(query) {
+        var regInfo = query.match(CURSOR_POS_EXP),
+            result;
+        
+        if (query.length <= 1 || !regInfo ||
+                (regInfo[1] && isNaN(regInfo[1])) ||
+                (regInfo[3] && isNaN(regInfo[3]))) {
+            
+            return null;
         }
-
-        return result;
+            
+        return {
+            query:  regInfo[0],
+            local:  query.indexOf(":") === 0,
+            line:   regInfo[1] - 1 || 0,
+            ch:     regInfo[3] - 1 || 0
+        };
     }
     
     /** Returns the last return value of _filterCallback(), which Smart Autocomplete helpfully caches */
@@ -312,18 +320,15 @@ define(function (require, exports, module) {
         }
         
         var selectedItem = domItemToSearchResult(selectedDOMItem),
-            doClose = true,
-            self = this;
+            doClose      = true,
+            self         = this,
+            query        = this.$searchField.val(),
+            cursorPos    = extractCursorPos(query);
 
         // Delegate to current plugin
         if (currentPlugin) {
             currentPlugin.itemSelect(selectedItem);
         } else {
-
-            // extract line number, if any
-            var query = this.$searchField.val(),
-                gotoLine = extractLineNumber(query);
-
             // Navigate to file and line number
             var fullPath = selectedItem && selectedItem.fullPath;
             if (fullPath) {
@@ -336,16 +341,16 @@ define(function (require, exports, module) {
                 this.modalBar.prepareClose();
                 CommandManager.execute(Commands.FILE_ADD_TO_WORKING_SET, {fullPath: fullPath})
                     .done(function () {
-                        if (!isNaN(gotoLine)) {
+                        if (cursorPos) {
                             var editor = EditorManager.getCurrentFullEditor();
-                            editor.setCursorPos(gotoLine, 0, true);
+                            editor.setCursorPos(cursorPos.line, cursorPos.ch, true);
                         }
                     })
                     .always(function () {
                         self.close();
                     });
-            } else if (!isNaN(gotoLine)) {
-                EditorManager.getCurrentFullEditor().setCursorPos(gotoLine, 0, true);
+            } else if (cursorPos) {
+                EditorManager.getCurrentFullEditor().setCursorPos(cursorPos.line, cursorPos.ch, true);
             }
         }
 
@@ -422,11 +427,11 @@ define(function (require, exports, module) {
             return true;
         }
         
-        var lineNum = extractLineNumber(query),
-            editor = EditorManager.getCurrentFullEditor();
+        var cursorPos = extractCursorPos(query),
+            editor    = EditorManager.getCurrentFullEditor();
         
         // We could just use 0 and lineCount() here, but in future we might want this logic to work for inline editors as well.
-        return (!isNaN(lineNum) && editor && lineNum >= editor.getFirstVisibleLine() && lineNum <= editor.getLastVisibleLine());
+        return (cursorPos && editor && cursorPos.line >= editor.getFirstVisibleLine() && cursorPos.line <= editor.getLastVisibleLine());
     };
     
     /**
@@ -545,6 +550,11 @@ define(function (require, exports, module) {
             return asyncResult.promise();
         }
         
+        var cursorPos = extractCursorPos(query);
+        if (cursorPos && !cursorPos.local && cursorPos.query !== "") {
+            query = query.replace(cursorPos.query, "");
+        }
+
         // First pass: filter based on search string; convert to SearchResults containing extra info
         // for sorting & display
         var filteredList = $.map(fileList, function (fileInfo) {
@@ -584,10 +594,10 @@ define(function (require, exports, module) {
         }
         
         // "Go to line" mode is special-cased
-        var gotoLine = extractLineNumber(query);
-        if (!isNaN(gotoLine)) {
-            var from = {line: gotoLine, ch: 0};
-            var to = {line: gotoLine, ch: 99999};
+        var cursorPos = extractCursorPos(query);
+        if (cursorPos && cursorPos.local) {
+            var from = {line: cursorPos.line, ch: cursorPos.ch},
+                to   = {line: cursorPos.line};
             
             EditorManager.getCurrentFullEditor().setSelection(from, to, true);
         }
diff --git a/test/spec/QuickOpen-test.js b/test/spec/QuickOpen-test.js
@@ -93,83 +93,103 @@ define(function (require, exports, module) {
             // on the QuickOpen input.
             SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_RETURN, "keyup", getSearchField()[0]);
         }
-
-        // TODO: fix me!
-        // This test is currently turned off due to failures on Windows 7
-        // See https://github.com/adobe/brackets/issues/2696
-        it("can open a file and jump to a line, centering that line on the screen", function () {
-            var err = false,
-                editor,
-                $scroller;
-            
-            SpecRunnerUtils.loadProjectInTestWindow(testPath);
-            
-            runs(function () {
-                var promise = SpecRunnerUtils.openProjectFiles([]);
-                waitsForDone(promise, "open project files");
-            });
-            
-            runs(function () {
-                // Test quick open using a partial file name
-                executeCommand(Commands.NAVIGATE_QUICK_OPEN);
+        
+        /**
+         * Creates a parameterized quick open test.
+         * @param {string} quickOpenQuery The search query to execute after the NAVIGATE_QUICK_OPEN command.
+         * @param {?string} gotoLineQuery The search query to execute after the NAVIGATE_GOTO_LINE command.
+         * @param {string} file The name of the file that should be opened.
+         * @param {number} line The line (1-based) where the cursor should be at the end of the operations.
+         * @param {number} col The column (1-based) where the cursor should be at the end of the operations.
+         * @return {function()} The configured test function.
+         */
+        function getQuickOpenTest(quickOpenQuery, gotoLineQuery, file, line, col) {
+            return function () {
+                var err = false,
+                    editor,
+                    $scroller;
                 
-                // need to set the timeout length here to ensure that it has a chance to load the file
-                // list.
-                enterSearchText("lines", 100);
-            });
-            
-            waitsFor(function () {
-                return getSearchField().val() === "lines";
-            }, "filename entry timeout", 1000);
-            
-            runs(function () {
-                pressEnter();
-            });
-            
-            waitsFor(function () {
-                editor = EditorManager.getCurrentFullEditor();
-                return editor !== null && getSearchBar().length === 0;
-            }, "file opening timeout", 3000);
-            
-            runs(function () {
-                $scroller = test$(editor.getScrollerElement());
-
-                // Make sure we've opened the right file. It should open the longer one, because
-                // of the scoring in the StringMatch algorithm.
-                expect(DocumentManager.getCurrentDocument().file.name).toEqual("lotsOfLines.html");
-
-                // Test go to line
-                executeCommand(Commands.NAVIGATE_GOTO_LINE);
-                enterSearchText(":50");
-            });
-            
-            waitsFor(function () {
-                return getSearchField().val() === ":50";
-            }, "goto line entry timeout", 1000);
-            
-            runs(function () {
-                pressEnter();
-            });
-
-            // wait for ModalBar to close
-            waitsFor(function () {
-                return getSearchBar().length === 0;
-            }, "ModalBar close", 1000);
-
-            runs(function () {
-                // The user enters a 1-based number, but the reported position
-                // is 0 based, so we check for 49.
-                expect(editor).toHaveCursorPosition(49, 0);
+                SpecRunnerUtils.loadProjectInTestWindow(testPath);
                 
-                // We expect the result to be scrolled roughly to the middle of the window.
-                var offset = $scroller.offset().top;
-                var editorHeight = $scroller.height();
-                var cursorPos = editor._codeMirror.cursorCoords(null, "page").bottom;
+                runs(function () {
+                    var promise = SpecRunnerUtils.openProjectFiles([]);
+                    waitsForDone(promise, "open project files");
+                });
                 
-                expect(cursorPos).toBeGreaterThan(editorHeight * 0.4 - offset);
-                expect(cursorPos).toBeLessThan(editorHeight * 0.6 - offset);
-            });
-        });
-
+                runs(function () {
+                    // Test quick open using a partial file name
+                    executeCommand(Commands.NAVIGATE_QUICK_OPEN);
+                    
+                    // need to set the timeout length here to ensure that it has a chance to load the file
+                    // list.
+                    enterSearchText(quickOpenQuery, 100);
+                });
+                
+                waitsFor(function () {
+                    return getSearchField().val() === quickOpenQuery;
+                }, "filename entry timeout", 1000);
+                
+                runs(function () {
+                    pressEnter();
+                });
+                
+                waitsFor(function () {
+                    editor = EditorManager.getCurrentFullEditor();
+                    return editor !== null && getSearchBar().length === 0;
+                }, "file opening timeout", 3000);
+                
+                runs(function () {
+                    $scroller = test$(editor.getScrollerElement());
+    
+                    // Make sure we've opened the right file. It should open the longer one, because
+                    // of the scoring in the StringMatch algorithm.
+                    expect(DocumentManager.getCurrentDocument().file.name).toEqual(file);
+    
+                    if (gotoLineQuery) {
+                        // Test go to line
+                        executeCommand(Commands.NAVIGATE_GOTO_LINE);
+                        enterSearchText(gotoLineQuery);
+                    }
+                });
+                
+                if (gotoLineQuery) {
+                    waitsFor(function () {
+                        return getSearchField().val() === gotoLineQuery;
+                    }, "goto line entry timeout", 1000);
+                    
+                    runs(function () {
+                        pressEnter();
+                    });
+    
+                    // wait for ModalBar to close
+                    waitsFor(function () {
+                        return getSearchBar().length === 0;
+                    }, "ModalBar close", 1000);
+                }
+    
+                runs(function () {
+                    // The user enters a 1-based number, but the reported position
+                    // is 0 based, so we check for line-1, col-1.
+                    expect(editor).toHaveCursorPosition(line - 1, col - 1);
+                    
+                    // We expect the result to be scrolled roughly to the middle of the window.
+                    var offset = $scroller.offset().top;
+                    var editorHeight = $scroller.height();
+                    var cursorPos = editor._codeMirror.cursorCoords(null, "page").bottom;
+                    
+                    expect(cursorPos).toBeGreaterThan(editorHeight * 0.4 - offset);
+                    expect(cursorPos).toBeLessThan(editorHeight * 0.6 - offset);
+                });
+            };
+        }
+        
+        it("can open a file and jump to a line, centering that line on the screen",
+            getQuickOpenTest("lines", ":50", "lotsOfLines.html", 50, 1));
+        
+        it("can open a file and jump to a line and column, centering that line on the screen",
+            getQuickOpenTest("lines", ":50,20", "lotsOfLines.html", 50, 20));
+        
+        it("can directly open a file in a given line and column, centering that line on the screen",
+            getQuickOpenTest("lines:150,20", null, "lotsOfLines.html", 150, 20));
     });
 });
