From 5156862b4b4027f219efc896e4756e3db9d219d9 Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Mon, 10 Feb 2025 12:47:20 -0500 Subject: [PATCH 1/5] Use object identity for simple editor position checks We already use object identity for this prop with Ace (and Redux helps us keep it stable) and this allows a person to click the link a second time and get the code re-selected. Since the prop doesn't change value, the second click does nothing with a value-based test. --- ui/frontend/editor/SimpleEditor.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ui/frontend/editor/SimpleEditor.tsx b/ui/frontend/editor/SimpleEditor.tsx index 19e8a15d..4393fe30 100644 --- a/ui/frontend/editor/SimpleEditor.tsx +++ b/ui/frontend/editor/SimpleEditor.tsx @@ -1,4 +1,3 @@ -import { isEqual } from 'lodash-es'; import React from 'react'; import { CommonEditorProps, Position, Selection } from '../types'; @@ -87,7 +86,7 @@ class SimpleEditor extends React.PureComponent { if (!newPosition || !editor) { return; } - if (isEqual(newPosition, oldPosition)) { + if (newPosition === oldPosition) { return; } @@ -104,7 +103,7 @@ class SimpleEditor extends React.PureComponent { if (!newSelection || !newSelection.start || !newSelection.end || !editor) { return; } - if (isEqual(newSelection, oldSelection)) { + if (newSelection === oldSelection) { return; } From 6feca9bbf9f22a76bc30fa3c5c8b71da204fd14c Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Mon, 10 Feb 2025 12:50:09 -0500 Subject: [PATCH 2/5] Fix going to a position in the simple editor with one line of code Off-by-one errors are the worst. --- ui/frontend/editor/SimpleEditor.tsx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/ui/frontend/editor/SimpleEditor.tsx b/ui/frontend/editor/SimpleEditor.tsx index 4393fe30..3e82cbe7 100644 --- a/ui/frontend/editor/SimpleEditor.tsx +++ b/ui/frontend/editor/SimpleEditor.tsx @@ -15,11 +15,9 @@ class CodeByteOffsets { public lineToOffsets(line: number) { const precedingBytes = this.bytesBeforeLine(line); + const nextPrecedingBytes = this.bytesBeforeLine(line + 1); - const highlightedLine = this.lines[line]; - const highlightedBytes = highlightedLine.length; - - return [precedingBytes, precedingBytes + highlightedBytes]; + return [precedingBytes, nextPrecedingBytes]; } public rangeToOffsets(start: Position, end: Position) { @@ -40,7 +38,7 @@ class CodeByteOffsets { const precedingLines = this.lines.slice(0, line); // Add one to account for the newline we split on and removed - return precedingLines.map((l) => l.length + 1).reduce((a, b) => a + b); + return precedingLines.map((l) => l.length + 1).reduce((a, b) => a + b, 0); } } From 50455faa2dab7a7fa63a9a74202b62b937f9573b Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Mon, 10 Feb 2025 13:08:41 -0500 Subject: [PATCH 3/5] Use `useCallback` to avoid running Monaco prop effects too much --- ui/frontend/editor/MonacoEditorCore.tsx | 176 +++++++++++++----------- 1 file changed, 98 insertions(+), 78 deletions(-) diff --git a/ui/frontend/editor/MonacoEditorCore.tsx b/ui/frontend/editor/MonacoEditorCore.tsx index 82f0bb90..0d6ca662 100644 --- a/ui/frontend/editor/MonacoEditorCore.tsx +++ b/ui/frontend/editor/MonacoEditorCore.tsx @@ -81,93 +81,113 @@ const MonacoEditorCore: React.FC = (props) => { editor.focus(); }, []); - useEditorProp(editor, props.onEditCode, (_editor, model, onEditCode) => { - model.onDidChangeContent(() => { - onEditCode(model.getValue()); - }); - }); - - useEditorProp(editor, props.execute, (editor, _model, execute) => { - editor.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyCode.Enter, () => { - execute(); - }); - // Ace's Vim mode runs code with :w, so let's do the same - editor.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyS, () => { - execute(); - }); - }); + useEditorProp( + editor, + props.onEditCode, + useCallback((_editor, model, onEditCode) => { + model.onDidChangeContent(() => { + onEditCode(model.getValue()); + }); + }, []), + ); - useEditorProp(editor, props.code, (editor, model, code) => { - // Short-circuit if nothing interesting to change. - if (code === model.getValue()) { - return; - } + useEditorProp( + editor, + props.execute, + useCallback((editor, _model, execute) => { + editor.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyCode.Enter, () => { + execute(); + }); + // Ace's Vim mode runs code with :w, so let's do the same + editor.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyS, () => { + execute(); + }); + }, []), + ); - editor.executeEdits('redux', [ - { - text: code, - range: model.getFullModelRange(), - }, - ]); - }); + useEditorProp( + editor, + props.code, + useCallback((editor, model, code) => { + // Short-circuit if nothing interesting to change. + if (code === model.getValue()) { + return; + } + + editor.executeEdits('redux', [ + { + text: code, + range: model.getFullModelRange(), + }, + ]); + }, []), + ); - useEditorProp(editor, theme, (editor, _model, theme) => { - editor.updateOptions({ theme }); - }); + useEditorProp( + editor, + theme, + useCallback((editor, _model, theme) => { + editor.updateOptions({ theme }); + }, []), + ); const autocompleteProps = useMemo( () => ({ autocompleteOnUse, crates: props.crates }), [autocompleteOnUse, props.crates], ); - useEditorProp(editor, autocompleteProps, (_editor, _model, { autocompleteOnUse, crates }) => { - completionProvider.current = monaco.languages.registerCompletionItemProvider('rust', { - triggerCharacters: [' '], - - provideCompletionItems(model, position, _context, _token) { - const word = model.getWordUntilPosition(position); - - function wordBefore( - word: monaco.editor.IWordAtPosition, - ): monaco.editor.IWordAtPosition | null { - const prevPos = { lineNumber: position.lineNumber, column: word.startColumn - 1 }; - return model.getWordAtPosition(prevPos); - } - - const preWord = wordBefore(word); - const prePreWord = preWord && wordBefore(preWord); - - const oldStyle = prePreWord?.word === 'extern' && preWord?.word === 'crate'; - const newStyle = autocompleteOnUse && preWord?.word === 'use'; - - const triggerPrefix = oldStyle || newStyle; - - if (!triggerPrefix) { - return { suggestions: [] }; - } - - const range = { - startLineNumber: position.lineNumber, - endLineNumber: position.lineNumber, - startColumn: word.startColumn, - endColumn: word.endColumn, - }; - - const suggestions = crates.map(({ name, version, id }) => ({ - kind: monaco.languages.CompletionItemKind.Module, - label: `${name} (${version})`, - insertText: `${id}; // ${version}`, - range, - })); - - return { suggestions }; - }, - }); - - return () => { - completionProvider.current?.dispose(); - }; - }); + useEditorProp( + editor, + autocompleteProps, + useCallback((_editor, _model, { autocompleteOnUse, crates }) => { + completionProvider.current = monaco.languages.registerCompletionItemProvider('rust', { + triggerCharacters: [' '], + + provideCompletionItems(model, position, _context, _token) { + const word = model.getWordUntilPosition(position); + + function wordBefore( + word: monaco.editor.IWordAtPosition, + ): monaco.editor.IWordAtPosition | null { + const prevPos = { lineNumber: position.lineNumber, column: word.startColumn - 1 }; + return model.getWordAtPosition(prevPos); + } + + const preWord = wordBefore(word); + const prePreWord = preWord && wordBefore(preWord); + + const oldStyle = prePreWord?.word === 'extern' && preWord?.word === 'crate'; + const newStyle = autocompleteOnUse && preWord?.word === 'use'; + + const triggerPrefix = oldStyle || newStyle; + + if (!triggerPrefix) { + return { suggestions: [] }; + } + + const range = { + startLineNumber: position.lineNumber, + endLineNumber: position.lineNumber, + startColumn: word.startColumn, + endColumn: word.endColumn, + }; + + const suggestions = crates.map(({ name, version, id }) => ({ + kind: monaco.languages.CompletionItemKind.Module, + label: `${name} (${version})`, + insertText: `${id}; // ${version}`, + range, + })); + + return { suggestions }; + }, + }); + + return () => { + completionProvider.current?.dispose(); + }; + }, []), + ); return
; }; From 83d5026a3e81fe0c6c1b57b8849d5e7389cb659b Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Mon, 10 Feb 2025 13:09:52 -0500 Subject: [PATCH 4/5] Support going to a line/column in the Monaco editor --- ui/frontend/editor/MonacoEditorCore.tsx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/ui/frontend/editor/MonacoEditorCore.tsx b/ui/frontend/editor/MonacoEditorCore.tsx index 0d6ca662..ca2f1822 100644 --- a/ui/frontend/editor/MonacoEditorCore.tsx +++ b/ui/frontend/editor/MonacoEditorCore.tsx @@ -189,6 +189,15 @@ const MonacoEditorCore: React.FC = (props) => { }, []), ); + useEditorProp( + editor, + props.position, + useCallback((editor, _model, { line, column }) => { + editor.setPosition({ lineNumber: line, column }); + editor.focus(); + }, []), + ); + return
; }; From 8d359daab444c219218f84030c5d1a51dcb3726b Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Mon, 10 Feb 2025 13:14:41 -0500 Subject: [PATCH 5/5] Allow typing over automatically-inserted closing symbols in Monaco --- ui/frontend/editor/MonacoEditorCore.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/frontend/editor/MonacoEditorCore.tsx b/ui/frontend/editor/MonacoEditorCore.tsx index ca2f1822..46530c0a 100644 --- a/ui/frontend/editor/MonacoEditorCore.tsx +++ b/ui/frontend/editor/MonacoEditorCore.tsx @@ -73,6 +73,7 @@ const MonacoEditorCore: React.FC = (props) => { fontFamily: nodeStyle.fontFamily, automaticLayout: true, 'semanticHighlighting.enabled': true, + autoClosingOvertype: 'always', }); setEditor(editor);