From 6a9d56f05ed80d1e2b09b9a022c35eb8fc3a63cc Mon Sep 17 00:00:00 2001 From: nightwing Date: Sat, 15 Apr 2023 13:58:12 +0400 Subject: [PATCH] cleanup --- ace.d.ts | 13 ++-- demo/kitchen-sink/demo.js | 19 +++-- demo/kitchen-sink/styles.css | 17 ++--- src/marker_group.js | 61 ++++++++++------ src/marker_group_test.js | 135 +++++++++++------------------------ src/search_highlight.js | 2 +- src/test/all_browser.js | 2 +- src/tooltip.js | 17 ++++- src/tooltip_test.js | 11 +++ 9 files changed, 131 insertions(+), 146 deletions(-) diff --git a/ace.d.ts b/ace.d.ts index 28c926a0610..b7d483857c7 100644 --- a/ace.d.ts +++ b/ace.d.ts @@ -227,6 +227,7 @@ export namespace Ace { value: string; session: EditSession; relativeLineNumbers: boolean; + enableMultiselect: boolean; enableKeyboardAccessibility: boolean; } @@ -275,17 +276,15 @@ export namespace Ace { type: string; } - export interface TooltipMarker { + export interface MarkerGroupItem { range: Range; - tooltipText: string; - className?: string; - priority?: number; + className: string; } export class MarkerGroup { - constructor(); - setMarkers: (markers: TooltipMarker[]) => void; - getMarkerAtPos: (pos: Position) => TooltipMarker; + constructor(session: EditSession); + setMarkers: (markers: MarkerGroupItem[]) => void; + getMarkerAtPosition: (pos: Position) => MarkerGroupItem; } diff --git a/demo/kitchen-sink/demo.js b/demo/kitchen-sink/demo.js index 35144001b95..ed48cf8e7c6 100644 --- a/demo/kitchen-sink/demo.js +++ b/demo/kitchen-sink/demo.js @@ -46,7 +46,6 @@ var ElasticTabstopsLite = require("ace/ext/elastic_tabstops_lite").ElasticTabsto var IncrementalSearch = require("ace/incremental_search").IncrementalSearch; var TokenTooltip = require("./token_tooltip").TokenTooltip; -var TooltipMarkerManager = require("ace/marker_group").TooltipMarkerManager; require("ace/config").defineOptions(Editor.prototype, "editor", { showTokenInfo: { set: function(val) { @@ -101,18 +100,21 @@ function loadLanguageProvider(editor) { }); window.languageProvider = languageProvider; languageProvider.registerEditor(editor); + // hack to replace tooltip implementation from ace-linters with hover tooltip + // can be removed when ace-linters is updated to use MarkerGroup and HoverTooltip if (languageProvider.$descriptionTooltip) editor.off("mousemove", languageProvider.$descriptionTooltip.onMouseMove); languageProvider.$messageController.$worker.addEventListener("message", function(e) { var id = e.data.sessionId.split(".")[0]; var session = languageProvider.$getSessionLanguageProvider({id: id})?.session; if (e.data.type == 6) { + // annotation message e.stopPropagation(); if (session) { showAnnotations(session, e.data.value); } } else if (e.data.type == 13) { - // ignore signatures + // highlights message if (session) showOccurrenceMarkers(session, e.data.value); } }, true); @@ -124,9 +126,12 @@ function loadLanguageProvider(editor) { var r = el.range; return { range: new Range(r.start.line, r.start.character, r.end.line, r.end.character), - className: el.kind == 3 - ? "language_highlight_occurrence_main" - : "language_highlight_occurrence_other" + // https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#documentHighlightKind + className: el.kind == 2 + ? "language_highlight_read" + : el.kind == 3 + ? "language_highlight_write" + : "language_highlight_text" }; })); } @@ -175,8 +180,8 @@ function loadLanguageProvider(editor) { }; var domNode = dom.buildDom(["div", {}, - hoverNode, - errorMarker && ["div", {}, errorMarker.tooltipText.trim()] + errorMarker && ["div", {}, errorMarker.tooltipText.trim()], + hoverNode ]); docTooltip.showForRange(editor, range, domNode, e); }); diff --git a/demo/kitchen-sink/styles.css b/demo/kitchen-sink/styles.css index d681144bf33..d669a716c4a 100644 --- a/demo/kitchen-sink/styles.css +++ b/demo/kitchen-sink/styles.css @@ -113,24 +113,15 @@ body { z-index: 2000; border-radius: 0; } -.language_highlight_occurrence_main { +.language_highlight_text, .language_highlight_read, .language_highlight_write { position: absolute; box-sizing: border-box; border: solid 1px #888; z-index: 2000; } -.ace_dark .language_highlight_occurrence_main { - border: solid 1px #888; -} -.language_highlight_occurrence_other { - position: absolute; - box-sizing: border-box; - border: solid 1px #888; - z-index: 2000; -} -.ace_dark .language_highlight_occurrence_other { - border: solid 1px #888; +.language_highlight_write { + border: solid 1px #F88; } -.ace_doc-tooltip pre { +.ace_tooltip pre { margin: 0; } \ No newline at end of file diff --git a/src/marker_group.js b/src/marker_group.js index 6aac0ec97cc..0e559c66cc5 100644 --- a/src/marker_group.js +++ b/src/marker_group.js @@ -2,9 +2,7 @@ /* Potential improvements: -- cap the length of rendered marker range -- save rendered markers and only search through rendered markers when looking for hover match -- don't cut off total amount of markers at 500 +- use binary search when looking for hover match */ class MarkerGroup { @@ -14,6 +12,11 @@ class MarkerGroup { session.addDynamicMarker(this); } + /** + * Finds the first marker containing pos + * @param {Position} pos + * @returns Ace.MarkerGroupItem + */ getMarkerAtPosition(pos) { return this.markers.find(function(marker) { return marker.range.contains(pos.row, pos.column); @@ -23,8 +26,8 @@ class MarkerGroup { /** * Comparator for Array.sort function, which sorts marker definitions by their positions * - * @param {Ace.TooltipMarker} a first marker. - * @param {Ace.TooltipMarker} b second marker. + * @param {Ace.MarkerGroupItem} a first marker. + * @param {Ace.MarkerGroupItem} b second marker. * @returns {number} negative number if a should be before b, positive number if b should be before a, 0 otherwise. */ markersComparator(a, b) { @@ -33,7 +36,7 @@ class MarkerGroup { /** * Sets marker definitions to be rendered. Limits the number of markers at MAX_MARKERS. - * @param {Ace.TooltipMarker[]} markers an array of marker definitions. + * @param {Ace.MarkerGroupItem[]} markers an array of marker definitions. */ setMarkers(markers) { this.markers = markers.sort(this.markersComparator).slice(0, this.MAX_MARKERS); @@ -44,9 +47,9 @@ class MarkerGroup { if (!this.markers || !this.markers.length) return; var visibleRangeStartRow = config.firstRow, visibleRangeEndRow = config.lastRow; - this.renderedMarkerRanges = {}; - - // TODO: if there are markers with overlapping ranges, do we merge them? if yes, how do we merge them? + var foldLine; + var markersOnOneLine = 0; + var lastRow = 0; for (var i = 0; i < this.markers.length; i++) { var marker = this.markers[i]; @@ -54,30 +57,46 @@ class MarkerGroup { if (marker.range.end.row < visibleRangeStartRow) continue; if (marker.range.start.row > visibleRangeEndRow) continue; + if (marker.range.start.row === lastRow) { + markersOnOneLine++; + } else { + lastRow = marker.range.start.row; + markersOnOneLine = 0; + } + // do not render too many markers on one line + // because we do not have virtual scroll for horizontal direction + if (markersOnOneLine > 200) { + continue; + } + var markerVisibleRange = marker.range.clipRows(visibleRangeStartRow, visibleRangeEndRow); if (markerVisibleRange.start.row === markerVisibleRange.end.row && markerVisibleRange.start.column === markerVisibleRange.end.column) { continue; // visible range is empty } - var rangeToAddMarkerTo = markerVisibleRange.toScreenRange(session); - var rangeAsString = rangeToAddMarkerTo.toString(); - if (this.renderedMarkerRanges[rangeAsString]) continue; + var screenRange = markerVisibleRange.toScreenRange(session); + if (screenRange.isEmpty()) { + // we are inside a fold + foldLine = session.getNextFoldLine(markerVisibleRange.end.row, foldLine); + if (foldLine && foldLine.end.row > markerVisibleRange.end.row) { + visibleRangeStartRow = foldLine.end.row; + } + continue; + } - this.renderedMarkerRanges[rangeAsString] = true; - if (rangeToAddMarkerTo.isMultiLine()) { - markerLayer.drawTextMarker(html, rangeToAddMarkerTo, marker.className, config); + if (screenRange.isMultiLine()) { + markerLayer.drawTextMarker(html, screenRange, marker.className, config); } else { - markerLayer.drawSingleLineMarker(html, rangeToAddMarkerTo, marker.className, config); + markerLayer.drawSingleLineMarker(html, screenRange, marker.className, config); } } - }; + } -}; +} -// this caps total amount of markers at 500 - should it maybe be done only for rendered markers? -// on top of it, do we need to cap the length of a rendered marker range to avoid performance issues? -MarkerGroup.prototype.MAX_MARKERS = 500; +// this caps total amount of markers at 10K +MarkerGroup.prototype.MAX_MARKERS = 10000; exports.MarkerGroup = MarkerGroup; diff --git a/src/marker_group_test.js b/src/marker_group_test.js index 8015bc5ec59..df88feea7d2 100644 --- a/src/marker_group_test.js +++ b/src/marker_group_test.js @@ -1,4 +1,3 @@ -/*global CustomEvent*/ if (typeof process !== "undefined") { require("./test/mockdom"); } @@ -6,121 +5,71 @@ if (typeof process !== "undefined") { "use strict"; var ace = require("./ace"); +var dom = require("./lib/dom"); var assert = require("./test/assertions"); var EditSession = require("./edit_session").EditSession; var Range = require("./range").Range; -var TooltipMarkerManager = require("./marker_group").TooltipMarkerManager; -var editor, tooltipMarkerManager; +var MarkerGroup = require("./marker_group").MarkerGroup; +var editor; var session1, session2; module.exports = { setUp: function(next) { - session1 = new EditSession(["Hello empty world", "This is a second line"]); - session2 = new EditSession(["Single line"]); + var value = "Hello empty world\n" + + "This is a second line" + + "\n".repeat(100) + + "line number 100"; + session1 = new EditSession(value); + session2 = new EditSession("2 " + value); editor = ace.edit(null, { session: session1 }); document.body.appendChild(editor.container); editor.container.style.height = "200px"; editor.container.style.width = "300px"; - tooltipMarkerManager = new TooltipMarkerManager(editor); - const singleLineMarker = { - range: new Range(0, 0, 0, 5), - tooltipText: "Single line marker", - className: "ace_tooltip-marker_test", - }; - const multiLineMarker = { - range: new Range(0, 12, 1, 4), - tooltipText: "Multi line marker", - className: "ace_tooltip-marker_test", - }; - - session1.setTooltipMarkers([singleLineMarker, multiLineMarker], tooltipMarkerManager); - - var css = '.ace_tooltip-marker_test { position: absolute; }', - head = document.head || document.getElementsByTagName('head')[0], - style = document.createElement('style'); - head.appendChild(style); - style.appendChild(document.createTextNode(css)); + dom.importCssString('.ace_tooltip-marker_test { position: absolute; }'); next(); }, - "test: show tooltip marker": function(next) { + "test: show markers": function() { editor.resize(true); - tooltipMarkerManager.hoverTooltip.idleTime = 3; - mouse("move", {row: 0, column: 1}); - setTimeout(function() { - var nodes = document.querySelectorAll(".ace_marker-tooltip"); - assert.equal(nodes.length, 1); - assert.equal(nodes[0].textContent, "Single line marker"); - assert.equal(tooltipMarkerManager.hoverTooltip.$element.style.display, "block"); - mouse("move", {row: 0, column: 9}); - setTimeout(function() { - assert.equal(tooltipMarkerManager.hoverTooltip.$element.style.display, "none"); - mouse("move", {row: 0, column: 15}); - setTimeout(function() { - assert.equal(tooltipMarkerManager.hoverTooltip.$element.style.display, "block"); - nodes = document.querySelectorAll(".ace_marker-tooltip"); - assert.equal(nodes.length, 1); - assert.equal(nodes[0].textContent, "Multi line marker"); - mouse("move", {row: 1, column: 1}); - setTimeout(function() { - assert.equal(tooltipMarkerManager.hoverTooltip.$element.style.display, "block"); - nodes = document.querySelectorAll(".ace_marker-tooltip"); - assert.equal(nodes.length, 1); - assert.equal(nodes[0].textContent, "Multi line marker"); - mouse("move", {row: 0, column: 9}); - next(); - }, 6); - }, 6); - }); - }, 6); - }, - "test: don't show tooltip in edit session without markers": function(next) { - editor.resize(true); - tooltipMarkerManager.hoverTooltip.idleTime = 3; - mouse("move", {row: 0, column: 1}); - setTimeout(function() { - var nodes = document.querySelectorAll(".ace_marker-tooltip"); - assert.equal(nodes.length, 1); - assert.equal(nodes[0].textContent, "Single line marker"); - assert.equal(tooltipMarkerManager.hoverTooltip.$element.style.display, "block"); - mouse("move", {row: 0, column: 9}); - editor.setSession(session2); - editor.resize(true); - mouse("move", {row: 0, column: 1}); - setTimeout(function() { - assert.equal(tooltipMarkerManager.hoverTooltip.$element.style.display, "none"); - next(); - }, 6); - }, 6); + editor.renderer.$loop._flush(); + var markerGroup = new MarkerGroup(session1); + + markerGroup.setMarkers([{ + range: new Range(0, 0, 0, 5), + className: "ace_tooltip-marker_test m2" + }, { + range: new Range(0, 12, 1, 4), + className: "ace_tooltip-marker_test m1", + isSecond: true + }]); + assert.ok(markerGroup.getMarkerAtPosition({row: 1, column: 1}).isSecond); + assert.ok(!markerGroup.getMarkerAtPosition({row: 3, column: 1})); + editor.renderer.$loop._flush(); + assert.equal(editor.container.querySelectorAll(".m1").length, 2); + assert.equal(editor.container.querySelectorAll(".m2").length, 1); + editor.setSession(session2); + editor.renderer.$loop._flush(); + assert.equal(editor.container.querySelectorAll(".m1").length, 0); + editor.setSession(session1); + editor.renderer.$loop._flush(); + assert.equal(editor.container.querySelectorAll(".m1").length, 2); + editor.execCommand("gotoend"); + editor.renderer.$loop._flush(); + assert.equal(editor.container.querySelectorAll(".m1").length, 0); + editor.execCommand("gotostart"); + editor.renderer.$loop._flush(); + assert.equal(editor.container.querySelectorAll(".m1").length, 2); + markerGroup.setMarkers([]); + editor.renderer.$loop._flush(); + assert.equal(editor.container.querySelectorAll(".m1").length, 0); }, tearDown: function() { editor.destroy(); - tooltipMarkerManager.destroy(); - editor = tooltipMarkerManager = null; } }; -function mouse(type, pos, properties) { - var target = editor.renderer.getMouseEventTarget(); - var event = new CustomEvent("mouse" + type, {bubbles: true}); - - if ("row" in pos) { - var pagePos = editor.renderer.textToScreenCoordinates(pos.row, pos.column); - event.clientX = pagePos.pageX; - event.clientY = pagePos.pageY; - } else { - target = pos; - var rect = target.getBoundingClientRect(); - event.clientX = rect.left + rect.width / 2; - event.clientY = rect.top + rect.height / 2; - } - Object.assign(event, properties); - target.dispatchEvent(event); -} - - if (typeof module !== "undefined" && module === require.main) { require("asyncjs").test.testcase(module.exports).exec(); } diff --git a/src/search_highlight.js b/src/search_highlight.js index ab6052090c3..8c941e82a51 100644 --- a/src/search_highlight.js +++ b/src/search_highlight.js @@ -47,7 +47,7 @@ class SearchHighlight { } } -}; +} // needed to prevent long lines from freezing the browser SearchHighlight.prototype.MAX_RANGES = 500; diff --git a/src/test/all_browser.js b/src/test/all_browser.js index b8dbc093dcd..c6143d6ade2 100644 --- a/src/test/all_browser.js +++ b/src/test/all_browser.js @@ -70,7 +70,7 @@ var testNames = [ "ace/search_test", "ace/selection_test", "ace/snippets_test", - "ace/tooltip_marker_test", + "ace/marker_group_test", "ace/tooltip_test", "ace/token_iterator_test", "ace/tokenizer_test", diff --git a/src/tooltip.js b/src/tooltip.js index eab709a9a52..39ac84baa5f 100644 --- a/src/tooltip.js +++ b/src/tooltip.js @@ -187,11 +187,22 @@ class HoverTooltip extends Tooltip { }.bind(this)); } - addToEditor(editor, callback, cancel) { + addToEditor(editor) { editor.on("mousemove", this.onMouseMove); + editor.on("mousedown", this.hide); editor.renderer.getMouseEventTarget().addEventListener("mouseout", this.onMouseOut, true); } - + + removeFromEditor(editor) { + editor.off("mousemove", this.onMouseMove); + editor.off("mousedown", this.hide); + editor.renderer.getMouseEventTarget().removeEventListener("mouseout", this.onMouseOut, true); + if (this.timeout) { + clearTimeout(this.timeout); + this.timeout = null; + } + } + onMouseMove(e, editor) { this.lastEvent = e; this.lastT = Date.now(); @@ -309,7 +320,7 @@ class HoverTooltip extends Tooltip { hide(e) { if (!e && document.activeElement == this.getElement()) return; - if (e && e.target && this.$element.contains(e.target)) + if (e && e.target && e.type != "keydown" && this.$element.contains(e.target)) return; this.lastEvent = null; clearTimeout(this.timeout); diff --git a/src/tooltip_test.js b/src/tooltip_test.js index 10b8202e702..09bcc4979de 100644 --- a/src/tooltip_test.js +++ b/src/tooltip_test.js @@ -102,6 +102,17 @@ module.exports = { }, 6); }, 6); }, 6); + }, + "test: remove listeners": function() { + var l = editor._eventRegistry.mousemove.length; + docTooltip.addToEditor(editor); + assert.ok(!docTooltip.timeout); + assert.equal(editor._eventRegistry.mousemove.length, l + 1); + mouse("move", {row: 0, column: 1}); + assert.ok(docTooltip.timeout); + docTooltip.removeFromEditor(editor); + assert.ok(!docTooltip.timeout); + assert.equal(editor._eventRegistry.mousemove.length, l); } };