Merge pull request #4463 from TomMalbran/issues-4332-4409

RaymondLim · RaymondLim · commit ace6a51714b2 · 2013-07-17T20:02:21.000-07:00
[OPEN] Fix #4332 and #4409: Sort issues fixed on the Working Set and Project Tree
diff --git a/src/file/FileUtils.js b/src/file/FileUtils.js
@@ -359,7 +359,7 @@ define(function (require, exports, module) {
     /**
      * Get the parent directory of a file. If a directory is passed in the directory is returned.
      * @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 
+     * @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);
@@ -393,6 +393,40 @@ define(function (require, exports, module) {
 
         return baseName.substr(idx);
     }
+    
+    /**
+     * @private
+     * Get the file name without the 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 extension ? filename.replace(new RegExp(extension + "$"), "") : filename;
+    }
+    
+    /**
+     * 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, extFirst) {
+        var ext1   = getFilenameExtension(filename1),
+            ext2   = getFilenameExtension(filename2),
+            cmpExt = ext1.toLocaleLowerCase().localeCompare(ext2.toLocaleLowerCase()),
+            cmpNames;
+        
+        if (brackets.platform === "win") {
+            filename1 = _getFilenameWithoutExtension(filename1);
+            filename2 = _getFilenameWithoutExtension(filename2);
+        }
+        cmpNames = filename1.toLocaleLowerCase().localeCompare(filename2.toLocaleLowerCase());
+        
+        return extFirst ? (cmpExt || cmpNames) : (cmpNames || cmpExt);
+    }
 
 
     // Define public API
@@ -417,4 +451,5 @@ define(function (require, exports, module) {
     exports.getDirectoryPath               = getDirectoryPath;
     exports.getBaseName                    = getBaseName;
     exports.getFilenameExtension           = getFilenameExtension;
+    exports.compareFilenames               = compareFilenames;
 });
diff --git a/src/project/ProjectManager.js b/src/project/ProjectManager.js
@@ -485,14 +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 = $(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
-                        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;
-                    } else {
-                        return this.get_text(a).toLowerCase() > this.get_text(b).toLowerCase() ? 1 : -1;
+                        a1 = ($(a).hasClass("jstree-leaf") ? "1" : "0") + a1;
+                        b1 = ($(b).hasClass("jstree-leaf") ? "1" : "0") + b1;
                     }
+                    return FileUtils.compareFilenames(a1, b1, false);
                 }
             }).bind(
                 "before.jstree",
@@ -764,6 +765,15 @@ define(function (require, exports, module) {
 
     }
     
+    /**
+     * Forces createNewItem() to complete by removing focus from the rename field which causes
+     * the new file to be written to disk
+     */
+    function forceFinishRename() {
+        $(".jstree-rename-input").blur();
+    }
+    
+    
     /** Returns the full path to the welcome project, which we open on first launch.
      * @private
      * @return {!string} fullPath reference
@@ -824,7 +834,6 @@ define(function (require, exports, module) {
      *  project is loaded and tree is rendered, or rejected if the project path
      *  fails to load.
      */
-
     function _loadProject(rootPath, isUpdating) {
         forceFinishRename();    // in case we're in the middle of renaming a file in the project
         
@@ -1516,13 +1525,6 @@ define(function (require, exports, module) {
         return result.promise();
     }
     
-    /**
-     * Forces createNewItem() to complete by removing focus from the rename field which causes
-     * the new file to be written to disk
-     */
-    function forceFinishRename() {
-        $(".jstree-rename-input").blur();
-    }
 
     // Initialize variables and listeners that depend on the HTML DOM
     AppInit.htmlReady(function () {
diff --git a/src/project/WorkingSetSort.js b/src/project/WorkingSetSort.js
@@ -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,29 +293,22 @@ define(function (require, exports, module) {
         function (file1, file2) {
             var index1 = DocumentManager.findInWorkingSetAddedOrder(file1.fullPath),
                 index2 = DocumentManager.findInWorkingSetAddedOrder(file2.fullPath);
+            
             return index1 - index2;
         },
         "workingSetAdd workingSetAddList"
     );
     register(
         Commands.SORT_WORKINGSET_BY_NAME,
         function (file1, file2) {
-            return file1.name.toLocaleLowerCase().localeCompare(file2.name.toLocaleLowerCase());
+            return FileUtils.compareFilenames(file1.name, file2.name, false);
         },
         "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);
-            
-            if (cmp === 0) {
-                return file1.name.toLocaleLowerCase().localeCompare(file2.name.toLocaleLowerCase());
-            } else {
-                return cmp;
-            }
+            return FileUtils.compareFilenames(file1.name, file2.name, true);
         },
         "workingSetAdd workingSetAddList"
     );
