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

Syntax for line:col jumps in QuickOpen search queries #5612

Merged
merged 6 commits into from
Jan 19, 2014
Merged

Syntax for line:col jumps in QuickOpen search queries #5612

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d6501e4
Generalizes cursor position in QuickOpen search queries
jbalsas Mar 24, 2013
67e3c82
Updates cursorPos regular expression
jbalsas May 2, 2013
f44f125
Add line,col quickopen tests
jbalsas Oct 21, 2013
a0708a1
Return null instead of just undefined
jbalsas Oct 21, 2013
82d3e40
Merge remote-tracking branch 'origin' into jbalsas/quickopen_cols
TomMalbran Jan 19, 2014
9290a6e
Nits fixed
TomMalbran Jan 19, 2014
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
71 changes: 41 additions & 30 deletions src/search/QuickOpen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ define(function (require, exports, module) {
ViewUtils = require("utils/ViewUtils");


var cursorPosRegExp = new RegExp(":([^,]+)?(,(.+)?)?");
Copy link
Contributor

Choose a reason for hiding this comment

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

This still requires Docs, to know what is this RegExp for, and to mark it as a constant.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or since is used only in extractCursorPos, maybe it could just be a variable there?


/** @type Array.<QuickOpenPlugin> */
var plugins = [];

Expand Down Expand Up @@ -249,22 +251,29 @@ 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
* @returns {{query: string, local: boolean, line: number, ch: number}} A cursorPos 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
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove the "cursorPos", since is not really that, but and object with the line and column position. It would also be nice to add that it returns null if there was a problem.

*/
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(cursorPosRegExp),
result;

if (query.length <= 1 ||
!regInfo ||
(regInfo[1] && isNaN(regInfo[1])) ||
(regInfo[3] && isNaN(regInfo[3]))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this is easier to read as:

if (query.length <= 1 || !regInfo ||
        (regInfo[1] && isNaN(regInfo[1])) || (regInfo[3] && isNaN(regInfo[3]))) {

Or in one line, to group the second or in one line.


return null;
}


result = {
query: regInfo[0],
local: query.indexOf(":") === 0,
line: regInfo[1] - 1 || 0,
ch: regInfo[3] - 1 || 0
};

return result;
}

Expand Down Expand Up @@ -311,17 +320,14 @@ define(function (require, exports, module) {

var selectedItem = domItemToSearchResult(selectedDOMItem),
doClose = true,
self = this;
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) {
Expand All @@ -334,16 +340,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);
}
}

Expand Down Expand Up @@ -420,11 +426,11 @@ define(function (require, exports, module) {
return true;
}

var lineNum = extractLineNumber(query),
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());
};

/**
Expand Down Expand Up @@ -541,6 +547,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) {
Expand Down Expand Up @@ -583,10 +594,10 @@ define(function (require, exports, module) {
this._updateDialogLabel(query);

// "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};
var to = {line: cursorPos.line};
Copy link
Contributor

Choose a reason for hiding this comment

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

Join the vars, and align the equals.


EditorManager.getCurrentFullEditor().setSelection(from, to, true);
}
Expand Down
170 changes: 95 additions & 75 deletions test/spec/QuickOpen-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
});
});