Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: color gutter optimize #1972

Merged
merged 1 commit into from
Dec 7, 2024
Merged

chore: color gutter optimize #1972

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 23 additions & 10 deletions src/editor/Editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2520,12 +2520,8 @@ define(function (require, exports, module) {
}
};

/**
* Renders all registered gutters
* @private
*/
Editor.prototype._renderGutters = function () {
var languageId = this.document.getLanguage().getId();
Editor.prototype._getRegisteredGutters = function () {
const languageId = this.document.getLanguage().getId();

function _filterByLanguages(gutter) {
return !gutter.languages || gutter.languages.indexOf(languageId) > -1;
Expand All @@ -2539,19 +2535,26 @@ define(function (require, exports, module) {
return gutter.name;
}

var gutters = registeredGutters.map(_getName),
rootElement = this.getRootElement();

// If the line numbers gutter has not been explicitly registered and the CodeMirror lineNumbes option is
// set to true, we explicitly add the line numbers gutter. This case occurs the first time the editor loads
// and showLineNumbers is set to true in preferences
const gutters = registeredGutters.map(_getName);
if (gutters.indexOf(LINE_NUMBER_GUTTER) < 0 && this._codeMirror.getOption(cmOptions[SHOW_LINE_NUMBERS])) {
registeredGutters.push({ name: LINE_NUMBER_GUTTER, priority: LINE_NUMBER_GUTTER_PRIORITY });
}

gutters = registeredGutters.sort(_sortByPriority)
return registeredGutters.sort(_sortByPriority)
.filter(_filterByLanguages)
.map(_getName);
};

/**
* Renders all registered gutters
* @private
*/
Editor.prototype._renderGutters = function () {
const rootElement = this.getRootElement();
const gutters = this._getRegisteredGutters();

this._codeMirror.setOption("gutters", gutters);
this._codeMirror.refresh();
Expand Down Expand Up @@ -2603,6 +2606,16 @@ define(function (require, exports, module) {
this.setGutterMarker(lineNumber, gutterName, null);
};

/**
* Returns true if this editor has the named gutter activated. gutters are considered active if the gutter is
* registered for the language of the file currently shown in the editor.
* @param {string} gutterName The name of the gutter to check
*/
Editor.prototype.isGutterActive = function (gutterName) {
const gutters = this._getRegisteredGutters();
return gutters.includes(gutterName);
};

/**
* Clears all marks from the gutter with the specified name.
* @param {string} gutterName The name of the gutter to clear.
Expand Down
52 changes: 7 additions & 45 deletions src/extensionsIntegrated/CSSColorPreview/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ define(function (require, exports, module) {
*/
function showColorMarks() {
if (!enabled) {
removeColorMarks();
return;
}

Expand Down Expand Up @@ -128,22 +127,6 @@ define(function (require, exports, module) {
});
}

/**
* To remove the color marks from the gutter of all the active editors
*/
function removeColorMarks() {

const allActiveEditors = MainViewManager.getAllViewedEditors();

allActiveEditors.forEach((activeEditor) => {
const currEditor = activeEditor.editor;
if(currEditor) {
currEditor.clearGutter(GUTTER_NAME);
}
});
}


/**
* To move the cursor to the color text and display the color quick edit
* @param {Editor} editor the codemirror instance
Expand Down Expand Up @@ -173,8 +156,8 @@ define(function (require, exports, module) {
* all the line numbers and the colors to be displayed on that line.
*/
function showGutters(editor, _results) {
// TODO we should show the gutter in those languages only if a color is present in that file.
if (editor && enabled) {
initGutter(editor);
const cm = editor._codeMirror;
editor.clearGutter(GUTTER_NAME); // clear color markers
_addDummyGutterMarkerIfNotExist(editor, editor.getCursorPos().line);
Expand Down Expand Up @@ -234,19 +217,6 @@ define(function (require, exports, module) {
}
}


/**
* Initialize the gutter
* @param {activeEditor} editor
*/
function initGutter(editor) {
if (!Editor.isGutterRegistered(GUTTER_NAME)) {
// we should restrict the languages here to Editor.registerGutter(..., ["css", "less", "scss", etc..]);
// TODO we should show the gutter in those languages only if a color is present in that file.
Editor.registerGutter(GUTTER_NAME, COLOR_PREVIEW_GUTTER_PRIORITY, COLOR_LANGUAGES);
}
}

function _addDummyGutterMarkerIfNotExist(editor, line) {
let marker = editor.getGutterMarker(line, GUTTER_NAME);
if(!marker){
Expand All @@ -270,8 +240,7 @@ define(function (require, exports, module) {

// Add listener for all editor changes
EditorManager.on("activeEditorChange", function (event, newEditor, oldEditor) {
if (newEditor) {
// todo: only attach if the color gutter is present as we disable it in certain languages
if (newEditor && newEditor.isGutterActive(GUTTER_NAME)) {
newEditor.off("cursorActivity.colorPreview");
newEditor.on("cursorActivity.colorPreview", _cursorActivity);
// Unbind the previous editor's change event if it exists
Expand All @@ -289,6 +258,7 @@ define(function (require, exports, module) {
}

showColorMarks();
_cursorActivity(null, newEditor);
}
});

Expand All @@ -311,9 +281,9 @@ define(function (require, exports, module) {
const value = PreferencesManager.get(PREFERENCES_CSS_COLOR_PREVIEW);
enabled = value;
if (!value) {
// to dynamically remove color to all active editors
removeColorMarks();
Editor.unregisterGutter(GUTTER_NAME);
} else {
Editor.registerGutter(GUTTER_NAME, COLOR_PREVIEW_GUTTER_PRIORITY, COLOR_LANGUAGES);
// to dynamically add color to all active editors
addColorMarksToAllEditors();
}
Expand All @@ -326,19 +296,11 @@ define(function (require, exports, module) {
showColorMarks();
}

/**
* Driver function, runs at the start of the program
*/
function init() {
// preferenceChanged calls 'showColorMarks' or 'removeColorMarks'
preferenceChanged();
registerHandlers();
}

// init after appReady
AppInit.appReady(function () {
PreferencesManager.on("change", PREFERENCES_CSS_COLOR_PREVIEW, preferenceChanged);
init();
preferenceChanged();
registerHandlers();
});
});

42 changes: 19 additions & 23 deletions test/spec/Editor-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2128,32 +2128,28 @@ define(function (require, exports, module) {
});

it("should return gutters registered with the same priority in insertion order", function () {
var secondRightGutter = "second-right";
const secondRightGutter = "second-right";
Editor.registerGutter(secondRightGutter, 101);
var expectedGutters = [leftGutter, lineNumberGutter, rightGutter, secondRightGutter];
var guttersInEditor = myEditor._codeMirror.getOption("gutters");
var registeredGutters = Editor.getRegisteredGutters().map(function (gutter) {
const expectedGutters = [leftGutter, lineNumberGutter, rightGutter, secondRightGutter];
const gutters = myEditor._codeMirror.getOption("gutters");
const registeredGutters = Editor.getRegisteredGutters().map(function (gutter) {
return gutter.name;
});
// registered color gutter is disabled in javascript files
expect(guttersInEditor.includes(colorGutter)).toBeFalse();
expect(registeredGutters.includes(colorGutter)).toBeTrue();
const allNonColorRegisteredGutters = registeredGutters.filter(function (gutter) {
return gutter !== colorGutter;
});
expect(guttersInEditor).toEqual(expectedGutters);
expect(allNonColorRegisteredGutters).toEqual(expectedGutters);
expect(gutters).toEqual(expectedGutters);
expect(registeredGutters).toEqual(expectedGutters);
});

it("should have only gutters registered with the intended languageIds ", function () {
var lessOnlyGutter = "less-only-gutter";
it("should have only gutters registered with the intended languageIds, test isGutterActive", function () {
const lessOnlyGutter = "less-only-gutter";
Editor.registerGutter(lessOnlyGutter, 101, ["less"]);
var expectedGutters = [leftGutter, lineNumberGutter, rightGutter];
var expectedRegisteredGutters = [leftGutter, lineNumberGutter, rightGutter, lessOnlyGutter, colorGutter];
var gutters = myEditor._codeMirror.getOption("gutters");
var registeredGutters = Editor.getRegisteredGutters().map(function (gutter) {
const expectedGutters = [leftGutter, lineNumberGutter, rightGutter];
const expectedRegisteredGutters = [leftGutter, lineNumberGutter, rightGutter, lessOnlyGutter];
const gutters = myEditor._codeMirror.getOption("gutters");
const registeredGutters = Editor.getRegisteredGutters().map(function (gutter) {
return gutter.name;
});
expect(myEditor.isGutterActive(lessOnlyGutter)).toBeFalse();
expect(myEditor.isGutterActive(leftGutter)).toBeTrue();
expect(gutters).toEqual(expectedGutters);
expect(registeredGutters).toEqual(expectedRegisteredGutters);
});
Expand All @@ -2162,13 +2158,13 @@ define(function (require, exports, module) {
Editor.unregisterGutter(leftGutter);
Editor.unregisterGutter(rightGutter);
Editor.registerGutter(leftGutter, 1);
var expectedGutters = [leftGutter, lineNumberGutter];
var guttersInEditor = myEditor._codeMirror.getOption("gutters");
var registeredGutters = Editor.getRegisteredGutters().map(function (gutter) {
const expectedGutters = [leftGutter, lineNumberGutter];
const gutters = myEditor._codeMirror.getOption("gutters");
const registeredGutters = Editor.getRegisteredGutters().map(function (gutter) {
return gutter.name;
});
expect(guttersInEditor).toEqual(expectedGutters); // js files dont have color gutter
expect(registeredGutters).toEqual([...expectedGutters, colorGutter]);
expect(gutters).toEqual(expectedGutters);
expect(registeredGutters).toEqual(expectedGutters);
});

it("should set gutter marker correctly", function () {
Expand Down
Loading