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

integration of code-folding extension into brackets #10792

Merged
merged 13 commits into from
Apr 7, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
116 changes: 116 additions & 0 deletions src/extensions/default/CodeFolding/Prefs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
/**
* Wrapper around brackets pref system to ensure preferences are stored in in one single object instead of using multiple keys.
* This is to make it easy for the user who edits their preferences file to easily manage the potentially numerous lines of preferences generated by the persisting code-folding state.
* @author Patrick Oladimeji
* @date 3/22/14 20:39:53 PM
*/
/*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50 */
/*global define, brackets*/
define(function (require, exports, module) {
"use strict";
var PreferencesManager = brackets.getModule("preferences/PreferencesManager"),
prefs = PreferencesManager.getExtensionPrefs("code-folding"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Any objection to renaming code-folding to maybe brackets-code-folding? Simply to helps everyone keep settings separate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in principle no objection. But I would suggest that I change the prefs in the version in the registry to "brackets-code-folding" for consistency - to match the name in the package.json.

Copy link
Contributor

Choose a reason for hiding this comment

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

That would be great. It would also help existing users keep saved settings from the extension.

Copy link
Contributor

Choose a reason for hiding this comment

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

@thehogfather Are you proposing that the integrated and the one in the registry should have the same preferences name? Or are you saying that instead of changing this integrated version we change the one in the registry?

What I expect is that users will remove the code folding extension when the new version of brackets comes out. At which point it doesn't really matter what we call the preferences. But if the plan is to continue to move the extension in the registry forward, then giving this integrated version the new name will let the users of the extension continue to use it without needing to migrate their settings.

Maybe I am lacking some understanding. @thehogfather is your plan to continue to develop the extension in the registry and have competing functionality with what we are integrating? Because I am not sure I see the purpose of having this integrated if the one in the registry will continue to provide the same functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MiguelCastillo no. Just proposing that the preference name should match the name in the package.json. So currently the version in the registry stores preferences using the "code-folding" key. I'll update it to "brackets-code-folding" and leave the version integrated into brackets as "code-folding". This also means any preferences saved in the old extension will be kept in the integrated version.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha. That makes sense. I was just curious if we wanted users to just transparently use the settings they have. In which case I expect them to remove the extension.

strings = brackets.getModule("strings"),
store = {},
foldsKey = "folds";

//default preference values
prefs.definePreference("enabled", "boolean", true,
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think any other Brackets code uses the fourth parameter options just yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No I didn't think so either. But I thought if the API exists why not use it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the extra meta data in there :)

{name: strings.ENABLE_CODE_FOLDING, description: strings.ENABLE_CODE_FOLDING});
prefs.definePreference("minFoldSize", "number", 2,
{name: strings.MIN_FOLD_SIZE, description: strings.MIN_FOLD_SIZE_HELP});
prefs.definePreference("saveFoldStates", "boolean", true,
{name: strings.SAVE_FOLD_STATES, description: strings.SAVE_FOLD_STATES_HELP});
prefs.definePreference("alwaysUseIndentFold", "boolean", true,
{name: strings.ALWAYS_USE_INDENT_FOLD, description: strings.ALWAYS_USE_INDENT_FOLD_HELP});
Copy link
Member

Choose a reason for hiding this comment

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

What does it mean to have this enabled? What is the behavior e.g. in a JS file if this conflicts with the standard brace-fold/comment-fold helpers?

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 means that line indentation is used as a suggestion for fold regions. Although this shouldn't take priority over range finders based on the current mode. So where there is conflict, the brace-fold-finder should take precedence. Having said that this might be another pref that could be better off disabled by default.

prefs.definePreference("enableRegionFolding", "boolean", true,
{name: strings.ENABLE_REGION_FOLDING, description: strings.ENABLE_REGION_FOLDING});
prefs.definePreference("fadeFoldButtons", "boolean", false,
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this preference should be on by default.

I can't actually get it to work though -- when I set it to true, it seems to have no effect (at least on JS or LESS files). Will file a bug.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, false alarm with the preferences issue.

I wonder if the best UX is actually in between this setting (hidden until mouseover) and the default (always visible) -- having the triangles be fainter until you mouse over the gutter, then getting darker. I may mock that up and see how it feels...

{name: strings.FADE_FOLD_BUTTONS, description: strings.FADE_FOLD_BUTTONS_HELP});
prefs.definePreference("maxFoldLevel", "number", 2,
{name: strings.MAX_FOLD_LEVEL, description: strings.MAX_FOLD_LEVEL_HELP});
prefs.definePreference("folds", "object", {});

/**
* Simplifies the fold ranges into an array of pairs of numbers.
* @param {!Object} folds the raw fold ranges indexed by line numbers
* @return {Object} an object whose keys are line numbers and the values are array
* of two 2-element arrays. First array contains [from.line, from.ch] and the second contains [to.line, to.ch]
*/
function simplify(folds) {
if (!folds) {
return;
}
var res = {}, range;
Object.keys(folds).forEach(function (line) {
range = folds[line];
res[line] = Array.isArray(range) ? range : [[range.from.line, range.from.ch], [range.to.line, range.to.ch]];
});
return res;
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

add a courtesy new line right here :D

* Inflates the fold ranges stored as simplified numeric arrays. The inflation converts the data into
* objects whose keys are line numbers and whose values are objects in the format {from: {line, ch}, to: {line, ch}}.
* @param {Object} folds the simplified fold ranges
* @return {Object} the converted fold ranges
*/
function inflate(folds) {
if (!folds) {
return;
}
//transform the folds into objects with from and to properties
var ranges = {}, obj;
Object.keys(folds).forEach(function (line) {
obj = folds[line];
ranges[line] = {from: {line: obj[0][0], ch: obj[0][1]}, to: {line: obj[1][0], ch: obj[1][1]}};
});

return ranges;
}

/**
* Gets the line folds saved for the specified path.
* @param {string} path the document path
* @return {Object} the line folds for the document at the specified path
*/
function getFolds(path) {
store = (prefs.get(foldsKey) || {});
return inflate(store[path]);
}

/**
* Saves the line folds for the specified path
* @param {!string} path the path to the document
* @param {Object} folds the fold ranges to save for the current document
*/
function setFolds(path, folds) {
store[path] = simplify(folds);
Copy link
Member

Choose a reason for hiding this comment

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

This really should be in view state and not the main preferences file -- it's exactly the kind of thing view state was intended for. This seems like something we should fix for 1.3 lest we start polluting everyone's preferences file (which should be easily human-readable) with folding state.

Copy link
Member

Choose a reason for hiding this comment

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

Filed as #10846 to make sure we track this

Copy link
Contributor

Choose a reason for hiding this comment

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

@thehogfather #10846 Instead of using the prefs , could we migrate the folding state to state.json?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@abose yes - for the folds. I suspect the other settings would need to be in the regular prefs (brackets.json).

Copy link
Contributor

Choose a reason for hiding this comment

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

@thehogfather could use PreferencesManager.getViewState("codefolding.states"); and PreferencesManager.setViewState("codefolding.states", state) to put to the state.json file.

prefs.set(foldsKey, store);
}

/**
* Get the code folding setting with the specified key from the store
* @param {!string} key The key for the setting to retrieve
* @return {string} the setting with the specified key
*/
function getSetting(key) {
return prefs.get(key);
}

/**
* Clears all the saved line folds for all documents.
*/
function clearAllFolds() {
prefs.set(foldsKey, {});
Copy link
Member

Choose a reason for hiding this comment

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

This function appears to be unused, but I think it also has a bug: this doesn't reset the store var used elsewhere, so any call to setFolds() later seems like it would immediately restore all the folds that had been cleared previously.

It's probably safest just to re-get the pref before setting it each time. There may be other bugs lurking with the current approach too (e.g. if you edit the prefs file... does it blow those changes away?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@peterflynn yes editing the prefs file for documents that are currently open in brackets (has no effect - the changes are restored one the document becomes the active editor). This is a bug - that your safe approach should fix.

}

module.exports.getFolds = getFolds;

module.exports.setFolds = setFolds;

module.exports.getSetting = getSetting;

module.exports.clearAllFolds = clearAllFolds;

});
268 changes: 268 additions & 0 deletions src/extensions/default/CodeFolding/foldhelpers/foldcode.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,268 @@
/**
* Based on http://codemirror.net/addon/fold/foldcode.js
* @author Patrick Oladimeji
Copy link
Member

Choose a reason for hiding this comment

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

I think we still need to keep the original copyright from CodeMirror here

* @date 10/28/13 8:41:46 AM
* @last modified 20 April 2014
*/
/*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50 */
/*global define, brackets, document*/
define(function (require, exports, module) {
"use strict";
var CodeMirror = brackets.getModule("thirdparty/CodeMirror2/lib/codemirror"),
prefs = require("Prefs");

/**
* Performs the folding and unfolding of code regions.
* @param {CodeMirror} cm the CodeMirror instance
* @param {number| Object} pos
*/
function doFold(cm, pos, options, force) {
options = options || {};
force = force || "fold";
if (typeof pos === "number") {
pos = CodeMirror.Pos(pos, 0);
}
var finder = options.rangeFinder || CodeMirror.fold.auto,
minSize = options.minFoldSize || prefs.getSetting("minFoldSize"),
range,
widget,
textRange;

function getRange(allowFolded) {
var range = options.range || finder(cm, pos);
if (!range || range.to.line - range.from.line < minSize) {
return null;
}
var marks = cm.findMarksAt(range.from),
i,
lastMark,
foldMarks;
for (i = 0; i < marks.length; ++i) {
if (marks[i].__isFold && force !== "fold") {
if (!allowFolded) {
return null;
}
range.cleared = true;
marks[i].clear();
}
}
//check for overlapping folds
if (marks && marks.length) {
foldMarks = marks.filter(function (d) {
return d.__isFold;
});
if (foldMarks && foldMarks.length) {
lastMark = foldMarks[foldMarks.length - 1].find();
if (lastMark && range.from.line <= lastMark.to.line && lastMark.to.line < range.to.line) {
return null;
}
}
}
return range;
}

function makeWidget() {
var widget = document.createElement("span");
widget.className = "CodeMirror-foldmarker";
return widget;
}

range = getRange(true);
if (options.scanUp) {
while (!range && pos.line > cm.firstLine()) {
pos = CodeMirror.Pos(pos.line - 1, 0);
range = getRange(false);
}
}
if (!range || range.cleared || force === "unfold" || range.to.line - range.from.line < minSize) {
if (range) { range.cleared = false; }
return;
}

widget = makeWidget();
textRange = cm.markText(range.from, range.to, {
replacedWith: widget,
clearOnEnter: true,
__isFold: true
});

CodeMirror.on(widget, "mousedown", function () {
textRange.clear();
});

textRange.on("clear", function (from, to) {
delete cm._lineFolds[from.line];
CodeMirror.signal(cm, "unfold", cm, from, to);
});

if (force === "fold") {
delete range.cleared;
cm._lineFolds[pos.line] = range;
} else {
delete cm._lineFolds[pos.line];
}

CodeMirror.signal(cm, force, cm, range.from, range.to);
return range;
}

/**
Initialises extensions and helpers on the CodeMirror object
*/
function init() {
CodeMirror.defineExtension("foldCode", function (pos, options, force) {
return doFold(this, pos, options, force);
});

CodeMirror.defineExtension("unfoldCode", function (pos, options) {
return doFold(this, pos, options, "unfold");
});

CodeMirror.defineExtension("isFolded", function (line) {
return this._lineFolds[line];
});

/**
* Checks the validity of the ranges passed in the parameter and returns the foldranges
* that are still valid in the current document
* @param {object} folds the dictionary of lines in the current document that should be folded
* @returns {object} valid folds found in those passed in parameter
*/
CodeMirror.defineExtension("getValidFolds", function (folds) {
var keys, rf = CodeMirror.fold.auto, cm = this, result = {}, range, cachedRange;
if (folds && (keys = Object.keys(folds)).length) {
keys.forEach(function (lineNumber) {
lineNumber = +lineNumber;
if (lineNumber >= cm.firstLine() && lineNumber <= cm.lastLine()) {
range = rf(cm, CodeMirror.Pos(lineNumber));
cachedRange = folds[lineNumber];
if (range && cachedRange && range.from.line === cachedRange.from.line &&
range.to.line === cachedRange.to.line) {
cm.foldCode(lineNumber, {range: folds[lineNumber]}, "fold");
result[lineNumber] = folds[lineNumber];
}
}
});
}
return result;
});

/**
* Utility function to fold the region at the current cursor position in a document
* @param {CodeMirror} cm the CodeMirror instance
* @param {?options} options extra options to pass to the fold function
*/
CodeMirror.commands.fold = function (cm, options) {
cm.foldCode(cm.getCursor(), options, "fold");
};

/**
* Utility function to unfold the region at the current cursor position in a document
* @param {CodeMirror} cm the CodeMirror instance
* @param {?options} options extra options to pass to the fold function
*/
CodeMirror.commands.unfold = function (cm, options) {
cm.foldCode(cm.getCursor(), options, "unfold");
};

/**
* Utility function to fold all foldable regions in a document
* @param {CodeMirror} cm the CodeMirror instance
*/
CodeMirror.commands.foldAll = function (cm) {
cm.operation(function () {
var i, e;
for (i = cm.firstLine(), e = cm.lastLine(); i <= e; i++) {
cm.foldCode(CodeMirror.Pos(i, 0), null, "fold");
}
});
};

/**
* Utility function to unfold all folded regions in a document
* @param {CodeMirror} cm the CodeMirror instance
* @param {?number} from the line number for the beginning of the region to unfold
* @param {?number} to the line number for the end of the region to unfold
*/
CodeMirror.commands.unfoldAll = function (cm, from, to) {
from = from || cm.firstLine();
to = to || cm.lastLine();
cm.operation(function () {
var i, e;
for (i = from, e = to; i <= e; i++) {
if (cm.isFolded(i)) { cm.unfoldCode(i, {range: cm._lineFolds[i]}); }
}
});
};

/**
* Folds the specified range. The descendants of any fold regions within the range are also folded up to
* a level set globally in the `maxFoldLevel' preferences
* @param {CodeMirror} cm the CodeMirror instance
* @param {?number} start the line number for the beginning of the region to fold
* @param {?number} end the line number for the end of the region to fold
*/
CodeMirror.commands.foldToLevel = function (cm, start, end) {
var rf = CodeMirror.fold.auto, level = prefs.getSetting("maxFoldLevel");
function foldLevel(n, from, to) {
if (n > 0) {
var i = from, range;
while (i < to) {
range = rf(cm, CodeMirror.Pos(i, 0));
if (range) {
//call fold level for the range just folded
foldLevel(n - 1, range.from.line + 1, range.to.line - 1);
cm.foldCode(CodeMirror.Pos(i, 0), null, "fold");
i = range.to.line + 1;
} else {
i++;
}
}
}
}
cm.operation(function () {
start = start === undefined ? cm.firstLine() : start;
end = end || cm.lastLine();
foldLevel(level, start, end);
});
};

/**
* Helper to combine an array of fold range finders into one
*/
CodeMirror.registerHelper("fold", "combine", function () {
var funcs = Array.prototype.slice.call(arguments, 0);
return function (cm, start) {
var i;
for (i = 0; i < funcs.length; ++i) {
var found = funcs[i] && funcs[i](cm, start);
if (found) {
return found;
}
}
};
});

/**
* Creates a helper which returns the appropriate fold function based on the mode of the current position in
* a document.
* @param {CodeMirror} cm the CodeMirror instance
* @param {number} start the current position in the document
*/
CodeMirror.registerHelper("fold", "auto", function (cm, start) {
var helpers = cm.getHelpers(start, "fold"), i, cur;
//ensure mode helper is loaded if there is one
var mode = cm.getMode().name;
var modeHelper = CodeMirror.fold[mode];
if (modeHelper && helpers.indexOf(modeHelper) < 0) {
helpers.push(modeHelper);
}
for (i = 0; i < helpers.length; i++) {
cur = helpers[i](cm, start);
if (cur) { return cur; }
}
});
}

exports.init = init;
});
Loading