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

New prefence for mac to sort the directories first in the project tree #7138

Merged
merged 3 commits into from
Mar 10, 2014
Merged

New prefence for mac to sort the directories first in the project tree #7138

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
51ca411
New prefence for mac to sort the directories first in the project tree
TomMalbran Mar 8, 2014
91e9621
Fixs after review
TomMalbran Mar 10, 2014
062fbd2
Improve performance
TomMalbran Mar 10, 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
35 changes: 24 additions & 11 deletions src/project/ProjectManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,21 +75,18 @@ define(function (require, exports, module) {
FileSyncManager = require("project/FileSyncManager"),
EditorManager = require("editor/EditorManager");


// Define the preference to decide how to sort the Project Tree files
PreferencesManager.definePreference("sortDirsFirst", "boolean", false);


/**
* @private
* Forward declaration for the _fileSystemChange and _fileSystemRename functions to make JSLint happy.
*/
var _fileSystemChange,
_fileSystemRename;

/**
* @private
* File tree sorting for mac-specific sorting behavior
*/
var _isMac = brackets.platform === "mac",
_sortPrefixDir = _isMac ? "" : "0",
_sortPrefixFile = _isMac ? "" : "1";

/**
* @private
Copy link
Contributor

Choose a reason for hiding this comment

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

This code works as desired, but I am concerned about performance. The PreferencesManager.get("sortDirectoriesFirst") function gets called on average O(n * log(n)) times for a list with n items, but it returns the same value every time. I think the original code is better -- only need to use preference instead of platform.

var _dirFirst       = PreferencesManager.get("sortDirectoriesFirst"),
    _sortPrefixDir  = _dirFirst ? "0" : "",
    _sortPrefixFile = _dirFirst ? "1" : "";

UPDATE: fixed original logic (was reversed)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, I am not sure how many operation PreferencesManager.get("sortDirectoriesFirst") does, but it might be better if it was saved as a variable or as you mention the prefixes too. I'll fix this.

* File and folder names which are not displayed or searched
Expand Down Expand Up @@ -689,6 +686,19 @@ define(function (require, exports, module) {
return fileName.match(_binaryExclusionListRegEx);
}

/**
* @private
* Returns the prefix added to the filename used for sorting
* @param {boolean} isFolder
* @return {string}
*/
function _getSortPrefix(isFolder) {
if (brackets.platform === "mac" && !PreferencesManager.get("sortDirsFirst")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this preference should not be Mac-only. User should be able to specify something like "directoriesFirst" or "alphabetical". If this preference is not specified, then the current sorting is used.

Do we need a separate pref for "ignoreCase"? Windows ignores case, but some systems may use strict ASCII order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be easy to just change the default value of this pref to false or true depending on the OS, so when defined by the user, it won't matter which OS they are using.

Currently we always ignore cases for any OS. Do you know of any OS or anyone who would like it to add an extra pref? Should that pref affect when we compare filenames in any operation, or just for the project tree?

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be easy to just change the default value of this pref to false or true depending on the OS

Sounds good.

Do you know of any OS or anyone who would like it to add an extra pref?

I haven't heard anyone request it, so that can be added later.

return "";
}
return isFolder ? "0" : "1";
}

/**
* @private
* Generate a string suitable for sorting
Expand All @@ -697,7 +707,7 @@ define(function (require, exports, module) {
* @return {string}
*/
function _toCompareString(name, isFolder) {
return ((isFolder) ? _sortPrefixDir : _sortPrefixFile) + name;
return _getSortPrefix(isFolder) + name;
}

/**
Expand Down Expand Up @@ -1469,7 +1479,7 @@ define(function (require, exports, module) {
if (typeof node === "string") {
node = {
data: node,
metadata: { compareString: _sortPrefixFile + node }
metadata: { compareString: _getSortPrefix(false) + node }
};
}

Expand Down Expand Up @@ -2187,6 +2197,9 @@ define(function (require, exports, module) {

$(exports).on("projectOpen", _reloadProjectPreferencesScope);

// Refresh the file tree when the sort pref changes
PreferencesManager.on("change", "sortDirsFirst", refreshFileTree);

// Event Handlers
$(FileViewController).on("documentSelectionFocusChange", _documentSelectionFocusChange);
$(FileViewController).on("fileViewFocusChange", _fileViewFocusChange);
Expand All @@ -2195,7 +2208,7 @@ define(function (require, exports, module) {
// Commands
CommandManager.register(Strings.CMD_OPEN_FOLDER, Commands.FILE_OPEN_FOLDER, openProject);
CommandManager.register(Strings.CMD_PROJECT_SETTINGS, Commands.FILE_PROJECT_SETTINGS, _projectSettings);
CommandManager.register(Strings.CMD_FILE_REFRESH, Commands.FILE_REFRESH, refreshFileTree);
CommandManager.register(Strings.CMD_FILE_REFRESH, Commands.FILE_REFRESH, refreshFileTree);

// Init invalid characters string
if (brackets.platform === "mac") {
Expand Down