From 46e90b21e9bf8eb61c54fa42c0c296696bced7c6 Mon Sep 17 00:00:00 2001 From: Raymond Hill Date: Sun, 2 Apr 2023 09:19:32 -0400 Subject: [PATCH] Code review of new linter code As per CodeMirror's documentation, eachLine() iterator is faster, so use this. Also no need to keep track or marked lines, we can just find them on demand, this makes the code simpler. --- src/js/codemirror/search.js | 35 ++++++------ src/js/codemirror/ubo-static-filtering.js | 69 ++++++++++++----------- 2 files changed, 56 insertions(+), 48 deletions(-) diff --git a/src/js/codemirror/search.js b/src/js/codemirror/search.js index 595345bfc..7f42b1f1b 100644 --- a/src/js/codemirror/search.js +++ b/src/js/codemirror/search.js @@ -144,7 +144,6 @@ import { i18n$ } from '../i18n.js'; this.queryTimer = null; this.dirty = true; this.lines = []; - this.errorLines = []; cm.on('changes', (cm, changes) => { for ( const change of changes ) { if ( change.text.length !== 0 || change.removed !== 0 ) { @@ -374,23 +373,28 @@ import { i18n$ } from '../i18n.js'; }; const findNextError = function(cm, dir) { - const state = getSearchState(cm); - const lines = state.errorLines; - if ( lines.length === 0 ) { return; } + const doc = cm.getDoc(); const cursor = cm.getCursor('from'); - const start = cursor.line; - const next = lines.reduce((best, v) => { + const cursorLine = cursor.line; + const start = dir < 0 ? 0 : cursorLine + 1; + const end = dir < 0 ? cursorLine : doc.lineCount(); + let found = -1; + doc.eachLine(start, end, lineHandle => { + const markers = lineHandle.gutterMarkers || null; + if ( markers === null ) { return; } + if ( markers['CodeMirror-lintgutter'] === undefined ) { return; } + const line = lineHandle.lineNo(); if ( dir < 0 ) { - if ( v < start && (best === -1 || v > best) ) { return v; } - return best; + found = line; + return; } - if ( v > start && (best === -1 || v < best) ) { return v; } - return best; - }, -1); - if ( next === -1 || next === start ) { return; } - cm.getDoc().setCursor(next); + found = line; + return true; + }); + if ( found === -1 || found === cursorLine ) { return; } + cm.getDoc().setCursor(found); const { clientHeight } = cm.getScrollInfo(); - cm.scrollIntoView({ line: next, ch: 0 }, clientHeight >>> 1); + cm.scrollIntoView({ line: found, ch: 0 }, clientHeight >>> 1); }; const clearSearch = function(cm, hard) { @@ -493,8 +497,7 @@ import { i18n$ } from '../i18n.js'; CodeMirror.defineInitHook(function(cm) { getSearchState(cm); cm.on('linterDone', details => { - const count = details.lines.length; - getSearchState(cm).errorLines = details.lines; + const count = details.errorCount; dom.cl.toggle('.cm-linter-widget', 'hasErrors', count !== 0); dom.text( '.cm-linter-widget .cm-linter-widget-count', diff --git a/src/js/codemirror/ubo-static-filtering.js b/src/js/codemirror/ubo-static-filtering.js index 5ea175af1..ba6b3dbe3 100644 --- a/src/js/codemirror/ubo-static-filtering.js +++ b/src/js/codemirror/ubo-static-filtering.js @@ -671,7 +671,7 @@ CodeMirror.registerHelper('fold', 'ubo-static-filtering', (( ) => { nativeCssHas: vAPI.webextFlavor.env.includes('native_css_has'), }); - const markedset = []; + let errorCount = 0; let markedsetStart = 0; let markedsetTimer; @@ -689,18 +689,14 @@ CodeMirror.registerHelper('fold', 'ubo-static-filtering', (( ) => { const line = markedsetStart++; const markers = lineHandle.gutterMarkers || null; if ( markers && markers['CodeMirror-lintgutter'] ) { - markedset.push(lineHandle.lineNo()); + errorCount += 1; } if ( (line & 0x0F) === 0 && deadline.timeRemaining() === 0 ) { processMarkedsetAsync(doc); return true; } if ( markedsetStart === lineCount ) { - CodeMirror.signal( - doc.getEditor(), - 'linterDone', - { lines: markedset } - ); + CodeMirror.signal(doc.getEditor(), 'linterDone', { errorCount }); } }); }; @@ -708,7 +704,7 @@ CodeMirror.registerHelper('fold', 'ubo-static-filtering', (( ) => { const changeset = []; let changesetTimer; - const addChanges = (doc, change) => { + const addChange = (doc, change) => { changeset.push(change); processChangesetAsync(doc); }; @@ -736,9 +732,10 @@ CodeMirror.registerHelper('fold', 'ubo-static-filtering', (( ) => { return error; }; - const extractMarker = lineInfo => { - if ( lineInfo.gutterMarkers instanceof Object === false ) { return; } - return lineInfo.gutterMarkers['CodeMirror-lintgutter']; + const extractMarker = lineHandle => { + const markers = lineHandle.gutterMarkers || null; + if ( markers === null ) { return; } + return markers['CodeMirror-lintgutter'] || undefined; }; const markerTemplate = (( ) => { @@ -750,41 +747,49 @@ CodeMirror.registerHelper('fold', 'ubo-static-filtering', (( ) => { return marker; })(); - const makeMarker = (doc, line, marker, error) => { + const makeMarker = (doc, lineHandle, marker, error) => { if ( marker === undefined ) { marker = markerTemplate.cloneNode(true); - doc.setGutterMarker(line, 'CodeMirror-lintgutter', marker); + doc.setGutterMarker(lineHandle, 'CodeMirror-lintgutter', marker); } marker.children[0].textContent = error; }; + const processChange = (doc, deadline, change) => { + let { from, to } = change; + doc.eachLine(from, to, lineHandle => { + astParser.parse(lineHandle.text); + const error = extractError(); + const marker = extractMarker(lineHandle); + if ( error === undefined && marker ) { + doc.setGutterMarker(lineHandle, 'CodeMirror-lintgutter', null); + } else if ( error !== undefined ) { + makeMarker(doc, lineHandle, marker, error); + } + from += 1; + if ( (from & 0x0F) !== 0 ) { return; } + if ( deadline.timeRemaining() !== 0 ) { return; } + return true; + }); + if ( from !== to ) { + return { from, to }; + } + }; + const processChangeset = (doc, deadline) => { const cm = doc.getEditor(); cm.startOperation(); while ( changeset.length !== 0 ) { - const { from, to } = changeset.shift(); - for ( let line = from; line < to; line++ ) { - const lineInfo = doc.lineInfo(line); - if ( lineInfo === null ) { continue; } - astParser.parse(lineInfo.text); - const error = extractError(); - let marker = extractMarker(lineInfo); - if ( error === undefined && marker ) { - doc.setGutterMarker(line, 'CodeMirror-lintgutter', null); - } else if ( error !== undefined ) { - makeMarker(doc, line, marker, error); - } - if ( (line & 0x0F) === 0 && deadline.timeRemaining() === 0 ) { - changeset.unshift({ doc, from: line+1, to }); - break; - } - } + const change = processChange(doc, deadline, changeset.shift()); + if ( change === undefined ) { continue; } + changeset.unshift(change); + break; } cm.endOperation(); if ( changeset.length !== 0 ) { return processChangesetAsync(doc); } - markedset.length = 0; + errorCount = 0; markedsetStart = 0; processMarkedsetAsync(doc); }; @@ -795,7 +800,7 @@ CodeMirror.registerHelper('fold', 'ubo-static-filtering', (( ) => { for ( const change of changes ) { const from = change.from.line; const to = from + change.text.length; - addChanges(doc, { from, to }); + addChange(doc, { from, to }); } }); });