cleanup

ajaxorg · Apr 18, 2023 · 6a9d56f · 6a9d56f
1 parent de81b71
commit 6a9d56f
Show file tree

Hide file tree

Showing 9 changed files with 131 additions and 146 deletions.
diff --git 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
@@ -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
@@ -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;
 }
diff --git 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,40 +47,56 @@ 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];
 
             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;