From a81d8c387034d6b0a54c360239d51e10ee9feb84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Malbr=C3=A1n?= Date: Mon, 15 Jul 2013 19:11:16 -0300 Subject: [PATCH 1/4] Sort issues fixed on the Working Set and Project Tree --- src/file/FileUtils.js | 18 ++++++++++++++++-- src/project/ProjectManager.js | 12 ++++++++---- src/project/WorkingSetSort.js | 29 +++++++++++++++++++++-------- 3 files changed, 45 insertions(+), 14 deletions(-) diff --git a/src/file/FileUtils.js b/src/file/FileUtils.js index f6035f7a6b3..42f7de67df9 100644 --- a/src/file/FileUtils.js +++ b/src/file/FileUtils.js @@ -359,13 +359,26 @@ define(function (require, exports, module) { /** * Get the parent directory of a file. If a directory is passed in the directory is returned. - * @param {string} full path to a file or directory - * @return {string} Returns the path to the parent directory of a file or the path of a directory + * @param {string} fullPath full path to a file or directory + * @return {string} Returns the path to the parent directory of a file or the path of a directory */ function getDirectoryPath(fullPath) { return fullPath.substr(0, fullPath.lastIndexOf("/") + 1); } + /** + * Get the file name without the extension and the file extension from a file name. + * @param {string} filename file name of a file or directory + * @return {{name: string, extension: string}} Returns the file real name and extension + */ + function getFileNameExtension(filename) { + var extension = _getFileExtension(filename), + name = filename.replace(new RegExp("." + extension + "$"), ""); + + return {name: name, extension: extension}; + } + + // Define public API exports.LINE_ENDINGS_CRLF = LINE_ENDINGS_CRLF; exports.LINE_ENDINGS_LF = LINE_ENDINGS_LF; @@ -386,4 +399,5 @@ define(function (require, exports, module) { exports.isStaticHtmlFileExt = isStaticHtmlFileExt; exports.isServerHtmlFileExt = isServerHtmlFileExt; exports.getDirectoryPath = getDirectoryPath; + exports.getFileNameExtension = getFileNameExtension; }); diff --git a/src/project/ProjectManager.js b/src/project/ProjectManager.js index 873447831b6..e653e1be8bb 100644 --- a/src/project/ProjectManager.js +++ b/src/project/ProjectManager.js @@ -485,14 +485,18 @@ define(function (require, exports, module) { //(note: our actual jsTree theme CSS lives in brackets.less; we specify an empty .css // file because jsTree insists on loading one itself) sort : function (a, b) { + var a1, b1, a2, b2; if (brackets.platform === "win") { // Windows: prepend folder names with a '0' and file names with a '1' so folders are listed first - var a1 = ($(a).hasClass("jstree-leaf") ? "1" : "0") + this.get_text(a).toLowerCase(), - b1 = ($(b).hasClass("jstree-leaf") ? "1" : "0") + this.get_text(b).toLowerCase(); - return (a1 > b1) ? 1 : -1; + a1 = ($(a).hasClass("jstree-leaf") ? "1" : "0") + $(a).text(); + b1 = ($(b).hasClass("jstree-leaf") ? "1" : "0") + $(b).text(); + a2 = FileUtils.getFileNameExtension(a1).name; + b2 = FileUtils.getFileNameExtension(b1).name; } else { - return this.get_text(a).toLowerCase() > this.get_text(b).toLowerCase() ? 1 : -1; + a2 = $(a).text(); + b2 = $(b).text(); } + return a2.toLocaleLowerCase().localeCompare(b2.toLocaleLowerCase()); } }).bind( "before.jstree", diff --git a/src/project/WorkingSetSort.js b/src/project/WorkingSetSort.js index ecbfb8d4048..c14f61f36fb 100644 --- a/src/project/WorkingSetSort.js +++ b/src/project/WorkingSetSort.js @@ -23,7 +23,7 @@ /*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50 */ -/*global define, $, window */ +/*global define, $, window, brackets */ /** * Manages the workingSet sort methods. @@ -35,6 +35,7 @@ define(function (require, exports, module) { CommandManager = require("command/CommandManager"), DocumentManager = require("document/DocumentManager"), PreferencesManager = require("preferences/PreferencesManager"), + FileUtils = require("file/FileUtils"), AppInit = require("utils/AppInit"), Strings = require("strings"); @@ -292,6 +293,7 @@ define(function (require, exports, module) { function (file1, file2) { var index1 = DocumentManager.findInWorkingSetAddedOrder(file1.fullPath), index2 = DocumentManager.findInWorkingSetAddedOrder(file2.fullPath); + return index1 - index2; }, "workingSetAdd workingSetAddList" @@ -299,21 +301,32 @@ define(function (require, exports, module) { register( Commands.SORT_WORKINGSET_BY_NAME, function (file1, file2) { - return file1.name.toLocaleLowerCase().localeCompare(file2.name.toLocaleLowerCase()); + var name1, name2; + if (brackets.platform === "win") { + name1 = FileUtils.getFileNameExtension(file1.name).name; + name2 = FileUtils.getFileNameExtension(file2.name).name; + } else { + name1 = file1.name; + name2 = file2.name; + } + return name1.toLocaleLowerCase().localeCompare(name2.toLocaleLowerCase()); }, "workingSetAdd workingSetAddList" ); register( Commands.SORT_WORKINGSET_BY_TYPE, function (file1, file2) { - var ext1 = file1.name.split('.').pop(), - ext2 = file2.name.split('.').pop(), - cmp = ext1.localeCompare(ext2); + var name1 = FileUtils.getFileNameExtension(file1.name), + name2 = FileUtils.getFileNameExtension(file2.name), + cmpExt = name1.extension.localeCompare(name2.extension); + + name1 = brackets.platform === "win" ? name1.name : file1.name; + name2 = brackets.platform === "win" ? name2.name : file2.name; - if (cmp === 0) { - return file1.name.toLocaleLowerCase().localeCompare(file2.name.toLocaleLowerCase()); + if (cmpExt === 0) { + return name1.toLocaleLowerCase().localeCompare(name2.toLocaleLowerCase()); } else { - return cmp; + return cmpExt; } }, "workingSetAdd workingSetAddList" From f65acfe3c9970317e190efbeffb8508aeaa866df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Malbr=C3=A1n?= Date: Wed, 17 Jul 2013 05:47:08 -0300 Subject: [PATCH 2/4] Fixes after review --- src/file/FileUtils.js | 29 +++++++++++++++++++++-------- src/project/ProjectManager.js | 17 +++++++---------- src/project/WorkingSetSort.js | 24 ++++++------------------ 3 files changed, 34 insertions(+), 36 deletions(-) diff --git a/src/file/FileUtils.js b/src/file/FileUtils.js index aa352e16888..e53a00603ec 100644 --- a/src/file/FileUtils.js +++ b/src/file/FileUtils.js @@ -395,15 +395,28 @@ define(function (require, exports, module) { } /** - * Get the file name without the extension and the file extension from a file name. + * @private + * Get the file name without the extension. * @param {string} filename file name of a file or directory - * @return {{name: string, extension: string}} Returns the file real name and extension + * @return {string} Returns the file real name and extension */ - function getFilenameAndExtension(filename) { - var extension = _getFileExtension(filename), - name = filename.replace(new RegExp("." + extension + "$"), ""); - - return {name: name, extension: extension}; + function _getFilenameName(filename) { + var extension = _getFileExtension(filename); + return filename.replace(new RegExp("." + extension + "$"), ""); + } + + /** + * Compares 2 filenames in lowercases. In Windows it compares the names without the extension + * @param {string} filename1 + * @param {string} filename2 + * @return {number} The result of the local compare function + */ + function compareFilenames(filename1, filename2) { + if (brackets.platform === "win") { + filename1 = _getFilenameName(filename1); + filename2 = _getFilenameName(filename2); + } + return filename1.toLocaleLowerCase().localeCompare(filename2.toLocaleLowerCase()); } @@ -429,5 +442,5 @@ define(function (require, exports, module) { exports.getDirectoryPath = getDirectoryPath; exports.getBaseName = getBaseName; exports.getFilenameExtension = getFilenameExtension; - exports.getFilenameAndExtension = getFilenameAndExtension; + exports.compareFilenames = compareFilenames; }); diff --git a/src/project/ProjectManager.js b/src/project/ProjectManager.js index 8de1826cb0c..f916de417bb 100644 --- a/src/project/ProjectManager.js +++ b/src/project/ProjectManager.js @@ -485,18 +485,15 @@ define(function (require, exports, module) { //(note: our actual jsTree theme CSS lives in brackets.less; we specify an empty .css // file because jsTree insists on loading one itself) sort : function (a, b) { - var a1, b1, a2, b2; + var a1 = $(a).text(), + b1 = $(b).text(); + + // Windows: prepend folder names with a '0' and file names with a '1' so folders are listed first if (brackets.platform === "win") { - // Windows: prepend folder names with a '0' and file names with a '1' so folders are listed first - a1 = ($(a).hasClass("jstree-leaf") ? "1" : "0") + $(a).text(); - b1 = ($(b).hasClass("jstree-leaf") ? "1" : "0") + $(b).text(); - a2 = FileUtils.getFilenameAndExtension(a1).name; - b2 = FileUtils.getFilenameAndExtension(b1).name; - } else { - a2 = $(a).text(); - b2 = $(b).text(); + a1 = ($(a).hasClass("jstree-leaf") ? "1" : "0") + a1; + b1 = ($(b).hasClass("jstree-leaf") ? "1" : "0") + b1; } - return a2.toLocaleLowerCase().localeCompare(b2.toLocaleLowerCase()); + return FileUtils.compareFilenames(a1, b1); } }).bind( "before.jstree", diff --git a/src/project/WorkingSetSort.js b/src/project/WorkingSetSort.js index 82352ed4b26..45f3605c464 100644 --- a/src/project/WorkingSetSort.js +++ b/src/project/WorkingSetSort.js @@ -301,33 +301,21 @@ define(function (require, exports, module) { register( Commands.SORT_WORKINGSET_BY_NAME, function (file1, file2) { - var name1, name2; - if (brackets.platform === "win") { - name1 = FileUtils.getFilenameAndExtension(file1.name).name; - name2 = FileUtils.getFilenameAndExtension(file2.name).name; - } else { - name1 = file1.name; - name2 = file2.name; - } - return name1.toLocaleLowerCase().localeCompare(name2.toLocaleLowerCase()); + return FileUtils.compareFilenames(file1.name, file2.name); }, "workingSetAdd workingSetAddList" ); register( Commands.SORT_WORKINGSET_BY_TYPE, function (file1, file2) { - var name1 = FileUtils.getFilenameAndExtension(file1.name), - name2 = FileUtils.getFilenameAndExtension(file2.name), - cmpExt = name1.extension.localeCompare(name2.extension); - - name1 = brackets.platform === "win" ? name1.name : file1.name; - name2 = brackets.platform === "win" ? name2.name : file2.name; + var ext1 = FileUtils.getFilenameExtension(file1.name), + ext2 = FileUtils.getFilenameExtension(file2.name), + cmpExt = ext1.toLocaleLowerCase().localeCompare(ext2.toLocaleLowerCase()); if (cmpExt === 0) { - return name1.toLocaleLowerCase().localeCompare(name2.toLocaleLowerCase()); - } else { - return cmpExt; + return FileUtils.compareFilenames(file1.name, file2.name); } + return cmpExt; }, "workingSetAdd workingSetAddList" ); From 49c48ba7501dceaa1d4aa23306912bd76585b247 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Malbr=C3=A1n?= Date: Wed, 17 Jul 2013 20:21:08 -0300 Subject: [PATCH 3/4] Fixes after second review --- src/file/FileUtils.js | 25 +++++++++++++++++-------- src/project/ProjectManager.js | 2 +- src/project/WorkingSetSort.js | 13 +++---------- 3 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/file/FileUtils.js b/src/file/FileUtils.js index e53a00603ec..ee21a5cd6cd 100644 --- a/src/file/FileUtils.js +++ b/src/file/FileUtils.js @@ -400,23 +400,32 @@ define(function (require, exports, module) { * @param {string} filename file name of a file or directory * @return {string} Returns the file real name and extension */ - function _getFilenameName(filename) { - var extension = _getFileExtension(filename); - return filename.replace(new RegExp("." + extension + "$"), ""); + function _getFilenameWithoutExtension(filename) { + var extension = getFilenameExtension(filename); + return filename.replace(new RegExp(extension + "$"), ""); } /** - * Compares 2 filenames in lowercases. In Windows it compares the names without the extension + * Compares 2 filenames in lowercases. In Windows it compares the names without the + * extension first and then the extensions to fix issue #4409 * @param {string} filename1 * @param {string} filename2 + * @param {boolean} extFirst If true it compares the extensions first and then the file names. * @return {number} The result of the local compare function */ - function compareFilenames(filename1, filename2) { + function compareFilenames(filename1, filename2, extFirst) { + var ext1 = getFilenameExtension(filename1), + ext2 = getFilenameExtension(filename2), + cmpExt = ext1.toLocaleLowerCase().localeCompare(ext2.toLocaleLowerCase()), + cmpNames; + if (brackets.platform === "win") { - filename1 = _getFilenameName(filename1); - filename2 = _getFilenameName(filename2); + filename1 = _getFilenameWithoutExtension(filename1); + filename2 = _getFilenameWithoutExtension(filename2); } - return filename1.toLocaleLowerCase().localeCompare(filename2.toLocaleLowerCase()); + cmpNames = filename1.toLocaleLowerCase().localeCompare(filename2.toLocaleLowerCase()); + + return extFirst ? (cmpExt || cmpNames) : (cmpNames || cmpExt); } diff --git a/src/project/ProjectManager.js b/src/project/ProjectManager.js index f916de417bb..293f9d6e590 100644 --- a/src/project/ProjectManager.js +++ b/src/project/ProjectManager.js @@ -493,7 +493,7 @@ define(function (require, exports, module) { a1 = ($(a).hasClass("jstree-leaf") ? "1" : "0") + a1; b1 = ($(b).hasClass("jstree-leaf") ? "1" : "0") + b1; } - return FileUtils.compareFilenames(a1, b1); + return FileUtils.compareFilenames(a1, b1, false); } }).bind( "before.jstree", diff --git a/src/project/WorkingSetSort.js b/src/project/WorkingSetSort.js index 45f3605c464..a38f5ed1dd3 100644 --- a/src/project/WorkingSetSort.js +++ b/src/project/WorkingSetSort.js @@ -23,7 +23,7 @@ /*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50 */ -/*global define, $, window, brackets */ +/*global define, $, window */ /** * Manages the workingSet sort methods. @@ -301,21 +301,14 @@ define(function (require, exports, module) { register( Commands.SORT_WORKINGSET_BY_NAME, function (file1, file2) { - return FileUtils.compareFilenames(file1.name, file2.name); + return FileUtils.compareFilenames(file1.name, file2.name, false); }, "workingSetAdd workingSetAddList" ); register( Commands.SORT_WORKINGSET_BY_TYPE, function (file1, file2) { - var ext1 = FileUtils.getFilenameExtension(file1.name), - ext2 = FileUtils.getFilenameExtension(file2.name), - cmpExt = ext1.toLocaleLowerCase().localeCompare(ext2.toLocaleLowerCase()); - - if (cmpExt === 0) { - return FileUtils.compareFilenames(file1.name, file2.name); - } - return cmpExt; + return FileUtils.compareFilenames(file1.name, file2.name, true); }, "workingSetAdd workingSetAddList" ); From 121ab4c8380f37f1c79b7a9214307f6cc15d745d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Malbr=C3=A1n?= Date: Wed, 17 Jul 2013 23:45:34 -0300 Subject: [PATCH 4/4] Fixes in getFilenameWithoutExtension --- src/file/FileUtils.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/file/FileUtils.js b/src/file/FileUtils.js index ee21a5cd6cd..824308643a3 100644 --- a/src/file/FileUtils.js +++ b/src/file/FileUtils.js @@ -397,12 +397,12 @@ define(function (require, exports, module) { /** * @private * Get the file name without the extension. - * @param {string} filename file name of a file or directory - * @return {string} Returns the file real name and extension + * @param {string} filename File name of a file or directory + * @return {string} Returns the file name without the extension */ function _getFilenameWithoutExtension(filename) { var extension = getFilenameExtension(filename); - return filename.replace(new RegExp(extension + "$"), ""); + return extension ? filename.replace(new RegExp(extension + "$"), "") : filename; } /**