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

Quick Edit result grouping #9614

Merged
merged 8 commits into from
Oct 23, 2014
88 changes: 39 additions & 49 deletions src/editor/CSSInlineEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ define(function (require, exports, module) {
DocumentManager = require("document/DocumentManager"),
EditorManager = require("editor/EditorManager"),
Editor = require("editor/Editor").Editor,
LanguageManager = require("language/LanguageManager"),
ProjectManager = require("project/ProjectManager"),
FileUtils = require("file/FileUtils"),
HTMLUtils = require("language/HTMLUtils"),
MultiRangeInlineEditor = require("editor/MultiRangeInlineEditor"),
Strings = require("strings"),
Expand Down Expand Up @@ -146,7 +148,7 @@ define(function (require, exports, module) {
/** Item renderer for stylesheet-picker dropdown */
function _stylesheetListRenderer(item) {
var html = "<span class='stylesheet-name'>" + _.escape(item.name);
if (item.subDirStr.length) {
if (item.subDirStr) {
html += "<span class='stylesheet-dir'> — " + _.escape(item.subDirStr) + "</span>";
}
html += "</span>";
Expand Down Expand Up @@ -240,65 +242,52 @@ define(function (require, exports, module) {

/**
* @private
* Sort fileInfo objects by name then sub-directory
* Sort files with LESS/SCSS above CSS, and then within each grouping sort by path & filename
* (the same order we use for Find in Files)
* @param {!File} a, b
* @return {number}
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

No @return annotation

function _sortFileInfos(a, b) {
var nameComparison = a.name.localeCompare(b.name);
if (nameComparison !== 0) {
return nameComparison;
function _fileComparator(a, b) {
var aIsCSS = LanguageManager.getLanguageForPath(a.fullPath).getId() === "css",
bIsCSS = LanguageManager.getLanguageForPath(b.fullPath).getId() === "css";
if (aIsCSS && !bIsCSS) {
return 1;
} else if (!aIsCSS && bIsCSS) {
return -1;
} else {
return FileUtils.comparePaths(a.fullPath, b.fullPath);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, the next version of JSLint flags an error when there is no default return statement (which I generally agree with), so I think this should be changed to:

            if (aIsCSS && !bIsCSS) {
                return 1;
            } else if (!aIsCSS && bIsCSS) {
                return -1;
            }
            return FileUtils.comparePaths(a.fullPath, b.fullPath);

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that's one of the reasons we chose not to update to that version :-) Personally, I prefer the structure the way it is so all return cases have matching indent...

return a.subDirStr.localeCompare(b.subDirStr);
}

/**
* @private
* Prepare file list for display
*/
function _prepFileList(fileInfos) {
var i, j, firstDupeIndex,
displayPaths = [],
dupeList = [];
function _prepFileList(files) {
// First, sort list (the same ordering we use for the results list)
files.sort(_fileComparator);

// Add subdir field to each entry
fileInfos.forEach(function (fileInfo) {
fileInfo.subDirStr = "";
});

// Add directory path to files with the same name so they can be
// distinguished in list. Start with list sorted by name.
fileInfos.sort(_sortFileInfos);

// For identical names, add a subdir
for (i = 1; i < fileInfos.length; i++) {
if (_sortFileInfos(fileInfos[i - 1], fileInfos[i]) === 0) {
// Duplicates found
firstDupeIndex = i - 1;
dupeList.push(fileInfos[i - 1]);
dupeList.push(fileInfos[i]);

// Lookahead for more dupes
while (++i < fileInfos.length &&
_sortFileInfos(dupeList[0], fileInfos[i]) === 0) {
dupeList.push(fileInfos[i]);
}

// Get minimum subdir to make each unique
displayPaths = ViewUtils.getDirNamesForDuplicateFiles(dupeList);

// Add a subdir to each dupe entry
for (j = 0; j < displayPaths.length; j++) {
fileInfos[firstDupeIndex + j].subDirStr = displayPaths[j];
}

// Release memory
dupeList = [];
// Find any files that share the same name (with different path)
var fileNames = {};
files.forEach(function (file) {
if (!fileNames[file.name]) {
fileNames[file.name] = [];
}
}
fileNames[file.name].push(file);
});

// Sort by name again, so paths are sorted
fileInfos.sort(_sortFileInfos);
// For any duplicate filenames, set subDirStr to a path snippet the helps
// the user distinguish each file in the list.
_.forEach(fileNames, function (files) {
if (files.length > 1) {
var displayPaths = ViewUtils.getDirNamesForDuplicateFiles(files);
files.forEach(function (file, i) {
file.subDirStr = displayPaths[i];
});
}
});

return fileInfos;
return files;
}

function _onHostEditorScroll() {
Expand All @@ -309,7 +298,8 @@ define(function (require, exports, module) {
.done(function (rules) {
var inlineEditorDeferred = new $.Deferred();
cssInlineEditor = new MultiRangeInlineEditor.MultiRangeInlineEditor(CSSUtils.consolidateRules(rules),
_getNoRulesMsg, CSSUtils.getRangeSelectors);
_getNoRulesMsg, CSSUtils.getRangeSelectors,
_fileComparator);
cssInlineEditor.load(hostEditor);
cssInlineEditor.$htmlContent
.on("focusin", _updateCommands)
Expand Down
2 changes: 1 addition & 1 deletion src/editor/Editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -1793,7 +1793,7 @@ define(function (require, exports, module) {
* Sets the height of an inline widget in this editor.
* @param {!InlineWidget} inlineWidget The widget whose height should be set.
* @param {!number} height The height of the widget.
* @param {boolean} ensureVisible Whether to scroll the entire widget into view.
* @param {boolean=} ensureVisible Whether to scroll the entire widget into view. Default false.
*/
Editor.prototype.setInlineWidgetHeight = function (inlineWidget, height, ensureVisible) {
var self = this,
Expand Down
9 changes: 4 additions & 5 deletions src/editor/InlineTextEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,8 @@ define(function (require, exports, module) {
/**
* Update the inline editor's height when the number of lines change. The
* base implementation of this method does nothing.
* @param {boolean} force the editor to resize
*/
InlineTextEditor.prototype.sizeInlineWidgetToContents = function (force) {
InlineTextEditor.prototype.sizeInlineWidgetToContents = function () {
// brackets_codemirror_overrides.css adds height:auto to CodeMirror
// Inline editors themselves do not need to be sized, but layouts like
// the one used in CSSInlineEditor do need some manual layout.
Expand Down Expand Up @@ -265,15 +264,15 @@ define(function (require, exports, module) {
// Always update the widget height when an inline editor completes a
// display update
$(this.editor).on("update.InlineTextEditor", function (event, editor) {
self.sizeInlineWidgetToContents(true);
self.sizeInlineWidgetToContents();
});

// Size editor to content whenever text changes (via edits here or any
// other view of the doc: Editor fires "change" any time its text
// changes, regardless of origin)
$(this.editor).on("change.InlineTextEditor", function (event, editor) {
if (self.hostEditor.isFullyVisible()) {
self.sizeInlineWidgetToContents(true);
self.sizeInlineWidgetToContents();
self._updateLineRange(editor);
}
});
Expand Down Expand Up @@ -326,7 +325,7 @@ define(function (require, exports, module) {
}

// We need to call this explicitly whenever the host editor is reshown
this.sizeInlineWidgetToContents(true);
this.sizeInlineWidgetToContents();
};

/**
Expand Down
Loading