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

Addresses issue #10846 #10850

Merged
merged 3 commits into from
Apr 10, 2015
Merged

Addresses issue #10846 #10850

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a66855e
Addresses issue #10846 - fold states are now persisted using viewStat…
thehogfather Apr 8, 2015
797ed3f
Removed string for collapse custom region menu.
thehogfather Apr 8, 2015
2c6a356
Moved preference strings from nls/lang to vars in Prefs.js
thehogfather Apr 9, 2015
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
49 changes: 30 additions & 19 deletions src/extensions/default/CodeFolding/Prefs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,27 +8,35 @@
/*global define, brackets*/
define(function (require, exports, module) {
"use strict";
var PreferencesManager = brackets.getModule("preferences/PreferencesManager"),
prefs = PreferencesManager.getExtensionPrefs("code-folding"),
strings = brackets.getModule("strings"),
store = {},
foldsKey = "folds";
var PreferencesManager = brackets.getModule("preferences/PreferencesManager"),
prefs = PreferencesManager.getExtensionPrefs("code-folding"),
foldsKey = "code-folding.folds",
// preference key strings are here for now since they are not used in any UI
ENABLE_CODE_FOLDING = "Enable code folding",
MIN_FOLD_SIZE = "Minimum fold size",
MIN_FOLD_SIZE_HELP = "Minimum number of lines to allow in a foldable range",
SAVE_FOLD_STATES = "Save fold states",
SAVE_FOLD_STATES_HELP = "Save fold states to disk when editor is closed and restore the folds when reopened",
ALWAYS_USE_INDENT_FOLD = "Always use indent fold",
ALWAYS_USE_INDENT_FOLD_HELP = "Fall back to using level of indentation as a folding guideline if no range finder is found for the current mode.",
FADE_FOLD_BUTTONS = "Fade fold buttons",
FADE_FOLD_BUTTONS_HELP = "Hides the fold buttons unless the mouse is over the gutter",
MAX_FOLD_LEVEL = "Max fold level",
MAX_FOLD_LEVEL_HELP = "Used to limit the number of nested folds to find and collapse when View -> Collapse All is called or Alt is held down when collapsing. Should improve performance for large files.";
Copy link
Member

Choose a reason for hiding this comment

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

This feels a little weird -- might be better for these to just be doc comments on the definePreference() calls for now... but not a big deal. I think this would be ok to merge as-is.


//default preference values
prefs.definePreference("enabled", "boolean", true,
{name: strings.ENABLE_CODE_FOLDING, description: strings.ENABLE_CODE_FOLDING});
{name: ENABLE_CODE_FOLDING, description: ENABLE_CODE_FOLDING});
prefs.definePreference("minFoldSize", "number", 2,
{name: strings.MIN_FOLD_SIZE, description: strings.MIN_FOLD_SIZE_HELP});
{name: MIN_FOLD_SIZE, description: 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});
prefs.definePreference("enableRegionFolding", "boolean", true,
{name: strings.ENABLE_REGION_FOLDING, description: strings.ENABLE_REGION_FOLDING});
{name: SAVE_FOLD_STATES, description: SAVE_FOLD_STATES_HELP});
prefs.definePreference("alwaysUseIndentFold", "boolean", false,
{name: ALWAYS_USE_INDENT_FOLD, description: ALWAYS_USE_INDENT_FOLD_HELP});
prefs.definePreference("fadeFoldButtons", "boolean", false,
{name: strings.FADE_FOLD_BUTTONS, description: strings.FADE_FOLD_BUTTONS_HELP});
{name: FADE_FOLD_BUTTONS, description: FADE_FOLD_BUTTONS_HELP});
prefs.definePreference("maxFoldLevel", "number", 2,
{name: strings.MAX_FOLD_LEVEL, description: strings.MAX_FOLD_LEVEL_HELP});
{name: MAX_FOLD_LEVEL, description: MAX_FOLD_LEVEL_HELP});
prefs.definePreference("folds", "object", {});
Copy link
Member

Choose a reason for hiding this comment

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

This should be turned into PreferencesManager.stateManager.definePreference(...


/**
Expand Down Expand Up @@ -75,8 +83,8 @@ define(function (require, exports, module) {
* @return {Object} the line folds for the document at the specified path
*/
function getFolds(path) {
store = (prefs.get(foldsKey) || {});
return inflate(store[path]);
var folds = (PreferencesManager.getViewState(foldsKey) || {});
return inflate(folds[path]);
}

/**
Expand All @@ -85,8 +93,9 @@ define(function (require, exports, module) {
* @param {Object} folds the fold ranges to save for the current document
*/
function setFolds(path, folds) {
store[path] = simplify(folds);
prefs.set(foldsKey, store);
var allFolds = PreferencesManager.getViewState(foldsKey);
Copy link
Member

Choose a reason for hiding this comment

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

You need || {} here too -- that's why saving folds is broken currently.

Copy link
Member

Choose a reason for hiding this comment

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

One other thought... rather than dumping folding state for all files together in one map, it might make sense to bucket it by project, like we do for other similar prefs like the scroll position & selection state of editors.

MainViewManager has an example of how to do that:

            context = { location : { scope: "user",
                                     layer: "project" } },
            state = PreferencesManager.getViewState(PREFS_NAME, context);

and

            context         = { location : { scope: "user",
                                         layer: "project",
                                         layerID: projectRoot.fullPath } },
        ...
        PreferencesManager.setViewState(PREFS_NAME, state, context);

Copy link
Contributor

Choose a reason for hiding this comment

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

@peterflynn can you make the || {} change in your PR?
Also issues#10869 added for tracking the bucketing by project.

Copy link
Contributor

Choose a reason for hiding this comment

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

brackets/issues/10870 opened to track Code Folding preference minFoldSize and alwaysUseIndentFold require close & reopen of file to take effect.

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 using the correct PreferencesManager.stateManager.definePreference(... will fix that, won't it?

Copy link
Member

Choose a reason for hiding this comment

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

@marcelgerber Yep, good point -- that should remove the need for || {} both here & in getFolds().

allFolds[path] = simplify(folds);
PreferencesManager.setViewState(foldsKey, allFolds);
}

/**
Expand All @@ -102,7 +111,7 @@ define(function (require, exports, module) {
* Clears all the saved line folds for all documents.
*/
function clearAllFolds() {
prefs.set(foldsKey, {});
PreferencesManager.setViewState(foldsKey, {});
}

module.exports.getFolds = getFolds;
Expand All @@ -113,4 +122,6 @@ define(function (require, exports, module) {

module.exports.clearAllFolds = clearAllFolds;

module.exports.prefBase = prefs;

});
14 changes: 8 additions & 6 deletions src/extensions/default/CodeFolding/foldhelpers/foldcode.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// CodeMirror, copyright (c) by Marijn Haverbeke and others
// Distributed under an MIT license: http://codemirror.net/LICENSE
/**
* Based on http://codemirror.net/addon/fold/foldcode.js
* @author Patrick Oladimeji
Expand All @@ -22,15 +24,15 @@ define(function (require, exports, module) {
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) {
if (!range || range.to.line - range.from.line < prefs.getSetting("minFoldSize")) {
return null;
}
var marks = cm.findMarksAt(range.from),
Expand Down Expand Up @@ -74,7 +76,7 @@ define(function (require, exports, module) {
range = getRange(false);
}
}
if (!range || range.cleared || force === "unfold" || range.to.line - range.from.line < minSize) {
if (!range || range.cleared || force === "unfold" || range.to.line - range.from.line < prefs.getSetting("minFoldSize")) {
if (range) { range.cleared = false; }
return;
}
Expand Down Expand Up @@ -119,7 +121,7 @@ define(function (require, exports, module) {
});

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

/**
Expand Down Expand Up @@ -203,7 +205,7 @@ define(function (require, exports, module) {
* @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");
var rf = CodeMirror.fold.auto;
function foldLevel(n, from, to) {
if (n > 0) {
var i = from, range;
Expand All @@ -223,7 +225,7 @@ define(function (require, exports, module) {
cm.operation(function () {
start = start === undefined ? cm.firstLine() : start;
end = end || cm.lastLine();
foldLevel(level, start, end);
foldLevel(prefs.getSetting("maxFoldLevel"), start, end);
});
};

Expand Down
7 changes: 4 additions & 3 deletions src/extensions/default/CodeFolding/foldhelpers/foldgutter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// CodeMirror, copyright (c) by Marijn Haverbeke and others
// Distributed under an MIT license: http://codemirror.net/LICENSE
/**
* Based on http://codemirror.net/addon/fold/foldgutter.js
* @author Patrick Oladimeji
Expand Down Expand Up @@ -220,7 +222,7 @@ define(function (require, exports, module) {
window.clearTimeout(state.changeUpdate);
state.changeUpdate = window.setTimeout(function () {
updateInViewport(cm);
}, prefs.getSetting("foldOnChangeTimeSpan") || 600);
}, 600);
}
}

Expand Down Expand Up @@ -251,7 +253,7 @@ define(function (require, exports, module) {
}
});
}
}, prefs.getSetting("updateViewportTimeSpan") || 400);
}, 400);
}

/**
Expand Down Expand Up @@ -310,7 +312,6 @@ define(function (require, exports, module) {
});
}


exports.init = init;
exports.clearGutter = clearGutter;
exports.updateInViewport = updateInViewport;
Expand Down
59 changes: 0 additions & 59 deletions src/extensions/default/CodeFolding/foldhelpers/latex-fold.js
View file
Open in desktop

This file was deleted.

66 changes: 0 additions & 66 deletions src/extensions/default/CodeFolding/foldhelpers/region-fold.js
View file
Open in desktop

This file was deleted.

Loading