From b464df36abf12f3ef602c7979c332973c4db8c45 Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Thu, 21 Nov 2024 16:30:59 -0500 Subject: [PATCH] CodeMirrorSearch: catch exceptions from invalid regex input Invalid regular expressions would error out on SearchQuery's getCursor() method. This is arguably an upstream bug, but we want to inform the user of invalid input anyway. We now show "Invalid regular expression" where the "$1 of $2" codemirror-find-results message is normally shown, and we add the error class to the Codex input. This is to be consistent with how the 2017 editor behaves. Also disable autocompletion which is more often distracting that helpful for a search field. Bump codemirror/search to include a fix where the selection isn't updated after a regex replacement. See https://discuss.codemirror.net/t/8832 Bug: T371436 Change-Id: I68722da98ef4925439caa64e8f3366031d56cf8e --- extension.json | 1 + i18n/en.json | 1 + i18n/qqq.json | 1 + package-lock.json | 8 ++--- package.json | 2 +- resources/codemirror.search.js | 25 ++++++++++++-- resources/lib/codemirror6.bundle.dist.js | 44 ++++++++++++++---------- 7 files changed, 56 insertions(+), 26 deletions(-) diff --git a/extension.json b/extension.json index 972316ce..3da3af38 100644 --- a/extension.json +++ b/extension.json @@ -242,6 +242,7 @@ "codemirror-prefs-title", "codemirror-previous", "codemirror-regexp", + "codemirror-regexp-invalid", "codemirror-replace", "codemirror-replace-all", "codemirror-replace-placeholder", diff --git a/i18n/en.json b/i18n/en.json index 9ebdab49..577d226f 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -30,6 +30,7 @@ "codemirror-all-tooltip": "Select all matches", "codemirror-match-case": "Match case", "codemirror-regexp": "Regular expression", + "codemirror-regexp-invalid": "Invalid regular expression", "codemirror-by-word": "By word", "codemirror-replace": "Replace", "codemirror-replace-placeholder": "Replace", diff --git a/i18n/qqq.json b/i18n/qqq.json index 42a5f783..ef74ed80 100644 --- a/i18n/qqq.json +++ b/i18n/qqq.json @@ -35,6 +35,7 @@ "codemirror-all-tooltip": "Tooltip shown when hovering over the 'All' button in the CodeMirror search panel.", "codemirror-match-case": "Tooltip for the 'Match case' button in the CodeMirror search panel.", "codemirror-regexp": "Tooltip for the 'Regular expression' button in the CodeMirror search panel.", + "codemirror-regexp-invalid": "Error message shown when the regular expression entered in the CodeMirror search panel is invalid.", "codemirror-by-word": "Tooltip for the 'By word' button in the CodeMirror search panel.", "codemirror-replace": "Label for the 'Replace' button in the CodeMirror search panel.", "codemirror-replace-placeholder": "Placeholder text for the 'Replace' input in the CodeMirror search panel.", diff --git a/package-lock.json b/package-lock.json index de6e822a..501316b5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,7 +9,7 @@ "@codemirror/autocomplete": "6.12.0", "@codemirror/commands": "6.2.5", "@codemirror/language": "6.9.3", - "@codemirror/search": "6.5.6", + "@codemirror/search": "6.5.8", "@codemirror/state": "6.2.1", "@codemirror/view": "6.22.2", "@lezer/highlight": "1.2.0", @@ -544,9 +544,9 @@ } }, "node_modules/@codemirror/search": { - "version": "6.5.6", - "resolved": "https://registry.npmjs.org/@codemirror/search/-/search-6.5.6.tgz", - "integrity": "sha512-rpMgcsh7o0GuCDUXKPvww+muLA1pDJaFrpq/CCHtpQJYz8xopu4D1hPcKRoDD0YlF8gZaqTNIRa4VRBWyhyy7Q==", + "version": "6.5.8", + "resolved": "https://registry.npmjs.org/@codemirror/search/-/search-6.5.8.tgz", + "integrity": "sha512-PoWtZvo7c1XFeZWmmyaOp2G0XVbOnm+fJzvghqGAktBW3cufwJUWvSCcNG0ppXiBEM05mZu6RhMtXPv2hpllig==", "dev": true, "dependencies": { "@codemirror/state": "^6.0.0", diff --git a/package.json b/package.json index 97124eea..fbd022c9 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,7 @@ "@codemirror/autocomplete": "6.12.0", "@codemirror/commands": "6.2.5", "@codemirror/language": "6.9.3", - "@codemirror/search": "6.5.6", + "@codemirror/search": "6.5.8", "@codemirror/state": "6.2.1", "@codemirror/view": "6.22.2", "@lezer/highlight": "1.2.0", diff --git a/resources/codemirror.search.js b/resources/codemirror.search.js index 67aaedcb..cd0843f4 100644 --- a/resources/codemirror.search.js +++ b/resources/codemirror.search.js @@ -37,6 +37,10 @@ class CodeMirrorSearch extends CodeMirrorPanel { * @type {HTMLInputElement} */ this.searchInput = undefined; + /** + * @type {HTMLDivElement} + */ + this.searchInputWrapper = undefined; /** * @type {HTMLInputElement} */ @@ -128,10 +132,11 @@ class CodeMirrorSearch extends CodeMirrorPanel { ); this.searchInput = searchInput; this.searchInput.setAttribute( 'main-field', 'true' ); + this.searchInputWrapper = searchInputWrapper; this.findResultsText = document.createElement( 'span' ); this.findResultsText.className = 'cm-mw-find-results'; - searchInputWrapper.appendChild( this.findResultsText ); - firstRow.appendChild( searchInputWrapper ); + this.searchInputWrapper.appendChild( this.findResultsText ); + firstRow.appendChild( this.searchInputWrapper ); this.appendPrevAndNextButtons( firstRow ); @@ -374,9 +379,24 @@ class CodeMirrorSearch extends CodeMirrorPanel { * @param {SearchQuery} [query] */ updateNumMatchesText( query ) { + if ( !!this.searchQuery.search && this.searchQuery.regexp && !this.searchQuery.valid ) { + this.searchInputWrapper.classList.add( 'cdx-text-input--status-error' ); + this.findResultsText.textContent = mw.msg( 'codemirror-regexp-invalid' ); + return; + } const cursor = query ? query.getCursor( this.view.state ) : getSearchQuery( this.view.state ).getCursor( this.view.state ); + + // Clear error state + this.searchInputWrapper.classList.remove( 'cdx-text-input--status-error' ); + + // Remove messaging if there's no search query. + if ( !this.searchQuery.search ) { + this.findResultsText.textContent = ''; + return; + } + let count = 0, current = 1; const { from, to } = this.view.state.selection.main; @@ -398,6 +418,7 @@ class CodeMirrorSearch extends CodeMirrorPanel { */ getTextInput( name, value = '', placeholder = '' ) { const [ container, input ] = super.getTextInput( name, value, placeholder ); + input.autocomplete = 'off'; input.addEventListener( 'change', this.commit.bind( this ) ); input.addEventListener( 'keyup', this.commit.bind( this ) ); return [ container, input ]; diff --git a/resources/lib/codemirror6.bundle.dist.js b/resources/lib/codemirror6.bundle.dist.js index 86976db3..170e1606 100644 --- a/resources/lib/codemirror6.bundle.dist.js +++ b/resources/lib/codemirror6.bundle.dist.js @@ -23449,19 +23449,20 @@ class SearchCursor { let str = fromCodePoint(next), start = this.bufferStart + this.bufferPos; this.bufferPos += codePointSize(next); let norm = this.normalize(str); - for (let i = 0, pos = start;; i++) { - let code = norm.charCodeAt(i); - let match = this.match(code, pos, this.bufferPos + this.bufferStart); - if (i == norm.length - 1) { - if (match) { - this.value = match; - return this; + if (norm.length) + for (let i = 0, pos = start;; i++) { + let code = norm.charCodeAt(i); + let match = this.match(code, pos, this.bufferPos + this.bufferStart); + if (i == norm.length - 1) { + if (match) { + this.value = match; + return this; + } + break; } - break; + if (pos == start && i < str.length && str.charCodeAt(i) == code) + pos++; } - if (pos == start && i < str.length && str.charCodeAt(i) == code) - pos++; - } } } match(code, pos, end) { @@ -24018,9 +24019,11 @@ class StringQuery extends QueryType { } nextMatch(state, curFrom, curTo) { let cursor = stringCursor(this.spec, state, curTo, state.doc.length).nextOverlapping(); - if (cursor.done) - cursor = stringCursor(this.spec, state, 0, curFrom).nextOverlapping(); - return cursor.done ? null : cursor.value; + if (cursor.done) { + let end = Math.min(state.doc.length, curFrom + this.spec.unquoted.length); + cursor = stringCursor(this.spec, state, 0, end).nextOverlapping(); + } + return cursor.done || cursor.value.from == curFrom && cursor.value.to == curTo ? null : cursor.value; } // Searching in reverse is, rather than implementing an inverted search // cursor, done by scanning chunk after chunk forward. @@ -24038,8 +24041,10 @@ class StringQuery extends QueryType { } } prevMatch(state, curFrom, curTo) { - return this.prevMatchInRange(state, 0, curFrom) || - this.prevMatchInRange(state, curTo, state.doc.length); + let found = this.prevMatchInRange(state, 0, curFrom); + if (!found) + found = this.prevMatchInRange(state, Math.max(0, curTo - this.spec.unquoted.length), state.doc.length); + return found && (found.from != curFrom || found.to != curTo) ? found : null; } getReplacement(_result) { return this.spec.unquote(this.spec.replace); } matchAll(state, limit) { @@ -24283,9 +24288,10 @@ const replaceNext = /*@__PURE__*/searchCommand((view, { query }) => { let { state } = view, { from, to } = state.selection.main; if (state.readOnly) return false; - let next = query.nextMatch(state, from, from); - if (!next) + let match = query.nextMatch(state, from, from); + if (!match) return false; + let next = match; let changes = [], selection, replacement; let effects = []; if (next.from == from && next.to == to) { @@ -24295,7 +24301,7 @@ const replaceNext = /*@__PURE__*/searchCommand((view, { query }) => { effects.push(EditorView.announce.of(state.phrase("replaced match on line $", state.doc.lineAt(from).number) + ".")); } if (next) { - let off = changes.length == 0 || changes[0].from >= next.to ? 0 : next.to - next.from - replacement.length; + let off = changes.length == 0 || changes[0].from >= match.to ? 0 : match.to - match.from - replacement.length; selection = EditorSelection.single(next.from - off, next.to - off); effects.push(announceMatch(view, next)); effects.push(state.facet(searchConfigFacet).scrollToMatch(selection.main, view));