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
This commit is contained in:
MusikAnimal 2024-11-21 16:30:59 -05:00
parent 4dfd50860f
commit b464df36ab
7 changed files with 56 additions and 26 deletions

View file

@ -242,6 +242,7 @@
"codemirror-prefs-title", "codemirror-prefs-title",
"codemirror-previous", "codemirror-previous",
"codemirror-regexp", "codemirror-regexp",
"codemirror-regexp-invalid",
"codemirror-replace", "codemirror-replace",
"codemirror-replace-all", "codemirror-replace-all",
"codemirror-replace-placeholder", "codemirror-replace-placeholder",

View file

@ -30,6 +30,7 @@
"codemirror-all-tooltip": "Select all matches", "codemirror-all-tooltip": "Select all matches",
"codemirror-match-case": "Match case", "codemirror-match-case": "Match case",
"codemirror-regexp": "Regular expression", "codemirror-regexp": "Regular expression",
"codemirror-regexp-invalid": "Invalid regular expression",
"codemirror-by-word": "By word", "codemirror-by-word": "By word",
"codemirror-replace": "Replace", "codemirror-replace": "Replace",
"codemirror-replace-placeholder": "Replace", "codemirror-replace-placeholder": "Replace",

View file

@ -35,6 +35,7 @@
"codemirror-all-tooltip": "Tooltip shown when hovering over the 'All' button in the CodeMirror search panel.", "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-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": "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-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": "Label for the 'Replace' button in the CodeMirror search panel.",
"codemirror-replace-placeholder": "Placeholder text for the 'Replace' input in the CodeMirror search panel.", "codemirror-replace-placeholder": "Placeholder text for the 'Replace' input in the CodeMirror search panel.",

8
package-lock.json generated
View file

@ -9,7 +9,7 @@
"@codemirror/autocomplete": "6.12.0", "@codemirror/autocomplete": "6.12.0",
"@codemirror/commands": "6.2.5", "@codemirror/commands": "6.2.5",
"@codemirror/language": "6.9.3", "@codemirror/language": "6.9.3",
"@codemirror/search": "6.5.6", "@codemirror/search": "6.5.8",
"@codemirror/state": "6.2.1", "@codemirror/state": "6.2.1",
"@codemirror/view": "6.22.2", "@codemirror/view": "6.22.2",
"@lezer/highlight": "1.2.0", "@lezer/highlight": "1.2.0",
@ -544,9 +544,9 @@
} }
}, },
"node_modules/@codemirror/search": { "node_modules/@codemirror/search": {
"version": "6.5.6", "version": "6.5.8",
"resolved": "https://registry.npmjs.org/@codemirror/search/-/search-6.5.6.tgz", "resolved": "https://registry.npmjs.org/@codemirror/search/-/search-6.5.8.tgz",
"integrity": "sha512-rpMgcsh7o0GuCDUXKPvww+muLA1pDJaFrpq/CCHtpQJYz8xopu4D1hPcKRoDD0YlF8gZaqTNIRa4VRBWyhyy7Q==", "integrity": "sha512-PoWtZvo7c1XFeZWmmyaOp2G0XVbOnm+fJzvghqGAktBW3cufwJUWvSCcNG0ppXiBEM05mZu6RhMtXPv2hpllig==",
"dev": true, "dev": true,
"dependencies": { "dependencies": {
"@codemirror/state": "^6.0.0", "@codemirror/state": "^6.0.0",

View file

@ -21,7 +21,7 @@
"@codemirror/autocomplete": "6.12.0", "@codemirror/autocomplete": "6.12.0",
"@codemirror/commands": "6.2.5", "@codemirror/commands": "6.2.5",
"@codemirror/language": "6.9.3", "@codemirror/language": "6.9.3",
"@codemirror/search": "6.5.6", "@codemirror/search": "6.5.8",
"@codemirror/state": "6.2.1", "@codemirror/state": "6.2.1",
"@codemirror/view": "6.22.2", "@codemirror/view": "6.22.2",
"@lezer/highlight": "1.2.0", "@lezer/highlight": "1.2.0",

View file

@ -37,6 +37,10 @@ class CodeMirrorSearch extends CodeMirrorPanel {
* @type {HTMLInputElement} * @type {HTMLInputElement}
*/ */
this.searchInput = undefined; this.searchInput = undefined;
/**
* @type {HTMLDivElement}
*/
this.searchInputWrapper = undefined;
/** /**
* @type {HTMLInputElement} * @type {HTMLInputElement}
*/ */
@ -128,10 +132,11 @@ class CodeMirrorSearch extends CodeMirrorPanel {
); );
this.searchInput = searchInput; this.searchInput = searchInput;
this.searchInput.setAttribute( 'main-field', 'true' ); this.searchInput.setAttribute( 'main-field', 'true' );
this.searchInputWrapper = searchInputWrapper;
this.findResultsText = document.createElement( 'span' ); this.findResultsText = document.createElement( 'span' );
this.findResultsText.className = 'cm-mw-find-results'; this.findResultsText.className = 'cm-mw-find-results';
searchInputWrapper.appendChild( this.findResultsText ); this.searchInputWrapper.appendChild( this.findResultsText );
firstRow.appendChild( searchInputWrapper ); firstRow.appendChild( this.searchInputWrapper );
this.appendPrevAndNextButtons( firstRow ); this.appendPrevAndNextButtons( firstRow );
@ -374,9 +379,24 @@ class CodeMirrorSearch extends CodeMirrorPanel {
* @param {SearchQuery} [query] * @param {SearchQuery} [query]
*/ */
updateNumMatchesText( 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 ? const cursor = query ?
query.getCursor( this.view.state ) : query.getCursor( this.view.state ) :
getSearchQuery( this.view.state ).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, let count = 0,
current = 1; current = 1;
const { from, to } = this.view.state.selection.main; const { from, to } = this.view.state.selection.main;
@ -398,6 +418,7 @@ class CodeMirrorSearch extends CodeMirrorPanel {
*/ */
getTextInput( name, value = '', placeholder = '' ) { getTextInput( name, value = '', placeholder = '' ) {
const [ container, input ] = super.getTextInput( name, value, placeholder ); const [ container, input ] = super.getTextInput( name, value, placeholder );
input.autocomplete = 'off';
input.addEventListener( 'change', this.commit.bind( this ) ); input.addEventListener( 'change', this.commit.bind( this ) );
input.addEventListener( 'keyup', this.commit.bind( this ) ); input.addEventListener( 'keyup', this.commit.bind( this ) );
return [ container, input ]; return [ container, input ];

View file

@ -23449,19 +23449,20 @@ class SearchCursor {
let str = fromCodePoint(next), start = this.bufferStart + this.bufferPos; let str = fromCodePoint(next), start = this.bufferStart + this.bufferPos;
this.bufferPos += codePointSize(next); this.bufferPos += codePointSize(next);
let norm = this.normalize(str); let norm = this.normalize(str);
for (let i = 0, pos = start;; i++) { if (norm.length)
let code = norm.charCodeAt(i); for (let i = 0, pos = start;; i++) {
let match = this.match(code, pos, this.bufferPos + this.bufferStart); let code = norm.charCodeAt(i);
if (i == norm.length - 1) { let match = this.match(code, pos, this.bufferPos + this.bufferStart);
if (match) { if (i == norm.length - 1) {
this.value = match; if (match) {
return this; 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) { match(code, pos, end) {
@ -24018,9 +24019,11 @@ class StringQuery extends QueryType {
} }
nextMatch(state, curFrom, curTo) { nextMatch(state, curFrom, curTo) {
let cursor = stringCursor(this.spec, state, curTo, state.doc.length).nextOverlapping(); let cursor = stringCursor(this.spec, state, curTo, state.doc.length).nextOverlapping();
if (cursor.done) if (cursor.done) {
cursor = stringCursor(this.spec, state, 0, curFrom).nextOverlapping(); let end = Math.min(state.doc.length, curFrom + this.spec.unquoted.length);
return cursor.done ? null : cursor.value; 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 // Searching in reverse is, rather than implementing an inverted search
// cursor, done by scanning chunk after chunk forward. // cursor, done by scanning chunk after chunk forward.
@ -24038,8 +24041,10 @@ class StringQuery extends QueryType {
} }
} }
prevMatch(state, curFrom, curTo) { prevMatch(state, curFrom, curTo) {
return this.prevMatchInRange(state, 0, curFrom) || let found = this.prevMatchInRange(state, 0, curFrom);
this.prevMatchInRange(state, curTo, state.doc.length); 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); } getReplacement(_result) { return this.spec.unquote(this.spec.replace); }
matchAll(state, limit) { matchAll(state, limit) {
@ -24283,9 +24288,10 @@ const replaceNext = /*@__PURE__*/searchCommand((view, { query }) => {
let { state } = view, { from, to } = state.selection.main; let { state } = view, { from, to } = state.selection.main;
if (state.readOnly) if (state.readOnly)
return false; return false;
let next = query.nextMatch(state, from, from); let match = query.nextMatch(state, from, from);
if (!next) if (!match)
return false; return false;
let next = match;
let changes = [], selection, replacement; let changes = [], selection, replacement;
let effects = []; let effects = [];
if (next.from == from && next.to == to) { 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) + ".")); effects.push(EditorView.announce.of(state.phrase("replaced match on line $", state.doc.lineAt(from).number) + "."));
} }
if (next) { 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); selection = EditorSelection.single(next.from - off, next.to - off);
effects.push(announceMatch(view, next)); effects.push(announceMatch(view, next));
effects.push(state.facet(searchConfigFacet).scrollToMatch(selection.main, view)); effects.push(state.facet(searchConfigFacet).scrollToMatch(selection.main, view));