-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Adds directory names to the files in working tree, ... #4419
Changes from 14 commits
c159aca
35b3668
988293c
46bcbd7
38519fc
6d89422
280b82a
693863e
bd79386
fd90174
76fbcac
e0945e2
a0e9923
f7778b7
d2cc6bb
da50ad0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,6 +39,7 @@ define(function (require, exports, module) { | |
Commands = require("command/Commands"), | ||
Menus = require("command/Menus"), | ||
FileViewController = require("project/FileViewController"), | ||
CollectionUtils = require("utils/CollectionUtils"), | ||
ViewUtils = require("utils/ViewUtils"); | ||
|
||
|
||
|
@@ -88,6 +89,112 @@ define(function (require, exports, module) { | |
} | ||
} | ||
|
||
/** | ||
* Adds directory names to elements representing passed files in working tree | ||
* @private | ||
* @param {Array.<FileEntry>} fileArray | ||
*/ | ||
function _addDirectoryNamesToWorkingTreeFiles(filesList) { | ||
// filesList must have at least two files in it for this to make sense | ||
if (filesList.length <= 1) { | ||
return; | ||
} | ||
|
||
// First collect paths from the list of files and fill map with them | ||
var map = {}, filePaths = [], displayPaths = []; | ||
filesList.forEach(function (file, index) { | ||
var fp = file.fullPath.split("/"); | ||
fp.pop(); // Remove the filename itself | ||
displayPaths[index] = fp.pop(); | ||
filePaths[index] = fp; | ||
|
||
if (!map[displayPaths[index]]) { | ||
map[displayPaths[index]] = [index]; | ||
} else { | ||
map[displayPaths[index]].push(index); | ||
} | ||
}); | ||
|
||
// This function is used to loop through map and resolve duplicate names | ||
var processMap = function (map) { | ||
var didSomething = false; | ||
CollectionUtils.forEach(map, function (arr, key) { | ||
// length > 1 means we have duplicates that need to be resolved | ||
if (arr.length > 1) { | ||
arr.forEach(function (index) { | ||
if (filePaths[index].length !== 0) { | ||
displayPaths[index] = filePaths[index].pop() + "/" + displayPaths[index]; | ||
didSomething = true; | ||
|
||
if (!map[displayPaths[index]]) { | ||
map[displayPaths[index]] = [index]; | ||
} else { | ||
map[displayPaths[index]].push(index); | ||
} | ||
} | ||
}); | ||
} | ||
delete map[key]; | ||
}); | ||
return didSomething; | ||
}; | ||
|
||
var repeat; | ||
do { | ||
repeat = processMap(map); | ||
} while (repeat); | ||
|
||
// Go through open files and add directories to appropriate entries | ||
$openFilesContainer.find("ul > li").each(function () { | ||
var $li = $(this); | ||
var io = filesList.indexOf($li.data(_FILE_KEY)); | ||
if (io !== -1) { | ||
var dirSplit = displayPaths[io].split("/"); | ||
if (dirSplit.length > 3) { | ||
displayPaths[io] = dirSplit[0] + "/\u2026/" + dirSplit[dirSplit.length - 1]; | ||
} | ||
|
||
var $dir = $("<span class='directory'/>").html(" — " + displayPaths[io]); | ||
$li.children("a").append($dir); | ||
} | ||
}); | ||
} | ||
|
||
/** | ||
* Looks for files with the same name in the working set | ||
* and adds a parent directory name to them | ||
* @private | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move @Private above the description |
||
*/ | ||
function _checkForDuplicatesInWorkingTree() { | ||
var map = {}, | ||
fileList = DocumentManager.getWorkingSet(); | ||
|
||
// we need to always clear current directories as files could be removed from working tree | ||
$openFilesContainer.find("ul > li > a > span.directory").remove(); | ||
|
||
// go through files and fill map with arrays of duplicates | ||
fileList.forEach(function (file) { | ||
// use the same function that is used to create html for file | ||
var displayHtml = ViewUtils.getFileEntryDisplay(file); | ||
|
||
if (map[displayHtml] === undefined) { | ||
map[displayHtml] = file; | ||
} else { | ||
if (!(map[displayHtml] instanceof Array)) { | ||
map[displayHtml] = [map[displayHtml]]; | ||
} | ||
map[displayHtml].push(file); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code (lines 180 to 187) could be simplified to something like:
Then at line 192 just check for |
||
}); | ||
|
||
// go through map and solve arrays, ignore rest | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A better rewording (after the previous change): |
||
CollectionUtils.forEach(map, function (value) { | ||
if (value instanceof Array) { | ||
_addDirectoryNamesToWorkingTreeFiles(value); | ||
} | ||
}); | ||
} | ||
|
||
/** | ||
* @private | ||
* Shows/Hides open files list based on working set content. | ||
|
@@ -99,6 +206,7 @@ define(function (require, exports, module) { | |
} else { | ||
$openFilesContainer.show(); | ||
$workingSetHeader.show(); | ||
_checkForDuplicatesInWorkingTree(); | ||
} | ||
_adjustForScrollbars(); | ||
_fireSelectionChanged(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,8 +41,7 @@ define(function (require, exports, module) { | |
|
||
var testPath = SpecRunnerUtils.getTestPath("/spec/WorkingSetView-test-files"), | ||
testWindow, | ||
workingSetCount; | ||
|
||
workingSetCount; | ||
|
||
function openAndMakeDirty(path) { | ||
var doc, didOpen = false, gotError = false; | ||
|
@@ -95,7 +94,6 @@ define(function (require, exports, module) { | |
SpecRunnerUtils.closeTestWindow(); | ||
} | ||
|
||
|
||
beforeFirst(function () { | ||
createTestWindow(this, true); | ||
}); | ||
|
@@ -116,8 +114,6 @@ define(function (require, exports, module) { | |
testWindow.closeAllFiles(); | ||
}); | ||
|
||
|
||
|
||
it("should add a list item when a file is dirtied", function () { | ||
// check if files are added to work set and dirty icons are present | ||
runs(function () { | ||
|
@@ -151,7 +147,7 @@ define(function (require, exports, module) { | |
runs(function () { | ||
var $ = testWindow.$; | ||
var secondItem = $($("#open-files-container > ul").children()[1]); | ||
secondItem.trigger('click'); | ||
secondItem.trigger("click"); | ||
|
||
var $listItems = $("#open-files-container > ul").children(); | ||
expect($($listItems[0]).hasClass("selected")).not.toBeTruthy(); | ||
|
@@ -219,7 +215,7 @@ define(function (require, exports, module) { | |
|
||
// hover over and click on close icon of 2nd list item | ||
var secondItem = $($("#open-files-container > ul").children()[1]); | ||
secondItem.trigger('mouseover'); | ||
secondItem.trigger("mouseover"); | ||
var closeIcon = secondItem.find(".file-status-icon"); | ||
expect(closeIcon.length).toBe(1); | ||
|
||
|
@@ -228,7 +224,7 @@ define(function (require, exports, module) { | |
didClose = true; | ||
}); | ||
|
||
closeIcon.trigger('mousedown'); | ||
closeIcon.trigger("mousedown"); | ||
}); | ||
|
||
waitsFor(function () { return didClose; }, "click on working set close icon timeout", 1000); | ||
|
@@ -257,7 +253,7 @@ define(function (require, exports, module) { | |
var $ = testWindow.$; | ||
var secondItem = $("#open-files-container > ul").children().eq(1); | ||
var fileName = secondItem.text(); | ||
secondItem.trigger('click'); | ||
secondItem.trigger("click"); | ||
|
||
// Calling FILE_RENAME synchronously works fine here since the item is already visible in project file tree. | ||
// However, if the selected item is not already visible in the tree, this command will complete asynchronously. | ||
|
@@ -269,6 +265,70 @@ define(function (require, exports, module) { | |
expect($projectFileItems.find("a.jstree-clicked").eq(0).siblings("input").eq(0).val()).toBe(fileName); | ||
}); | ||
}); | ||
|
||
it("should show a directory name next to the file name when two files with same names are opened", function () { | ||
runs(function() { | ||
// Count currently opened files | ||
var workingSetCountBeforeTest = workingSetCount; | ||
|
||
// First we need to open another file | ||
openAndMakeDirty(testPath + "/directory/file_one.js"); | ||
|
||
// Wait for file to be added to the working set | ||
waitsFor(function () { return workingSetCount === workingSetCountBeforeTest + 1; }, 1000); | ||
|
||
runs(function() { | ||
// Two files with the same name file_one.js should be now opened | ||
var $list = testWindow.$("#open-files-container > ul"); | ||
expect($list.find(".directory").length).toBe(2); | ||
|
||
// Now close last opened file to hide the directories again | ||
DocumentManager.getCurrentDocument()._markClean(); // so we can close without a save dialog | ||
var didClose = false, gotError = false; | ||
CommandManager.execute(Commands.FILE_CLOSE) | ||
.done(function () { didClose = true; }) | ||
.fail(function () { gotError = true; }); | ||
waitsFor(function () { return didClose && !gotError; }, "timeout on FILE_CLOSE", 1000); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can replace from line 287 to line 291 with: |
||
|
||
// there should be no more directories shown | ||
runs(function () { | ||
expect($list.find(".directory").length).toBe(0); | ||
}); | ||
}); | ||
}); | ||
}); | ||
|
||
it("should show different directory names, when two files of the same name are opened, located in folders with same name", function () { | ||
runs(function () { | ||
// Count currently opened files | ||
var workingSetCountBeforeTest = workingSetCount; | ||
|
||
// Open both files | ||
openAndMakeDirty(testPath + "/directory/file_one.js"); | ||
openAndMakeDirty(testPath + "/directory/directory/file_one.js"); | ||
|
||
// Wait for them to load | ||
waitsFor(function () { return workingSetCount === workingSetCountBeforeTest + 2; }, 1000); | ||
|
||
runs(function() { | ||
// Collect all directory names displayed | ||
var $list = testWindow.$("#open-files-container > ul"); | ||
var names = $list.find(".directory").map(function () { | ||
return $(this).text(); | ||
}).toArray(); | ||
|
||
// All directory names should be unique | ||
var uniq = 0, map = {}; | ||
names.forEach(function(name) { | ||
if (!map[name]) { | ||
map[name] = true; | ||
uniq++; | ||
} | ||
}); | ||
expect(uniq).toBe(names.length); | ||
}); | ||
}); | ||
}); | ||
|
||
}); | ||
}); |
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.
Move the
@private
above the description.Rename
fileArray
tofilesList
and add a description since it kinds of expects FileEntries with the same filenames. Would work with any array of FileEntries but will not do the same stuff.