From c618a19fc7fcdb6147c27784075802fb0a3364de Mon Sep 17 00:00:00 2001 From: Matthew Runyon Date: Tue, 5 Nov 2024 16:22:36 -0600 Subject: [PATCH 1/4] feat: Ruff updates for DHE support --- .../console/src/monaco/MonacoProviders.tsx | 28 ++----- packages/console/src/monaco/MonacoUtils.ts | 40 +++++++++ .../console/src/monaco/RuffSettingsModal.tsx | 81 +++++++++++-------- .../src/ConsolePlugin.tsx | 17 +--- packages/dashboard-core-plugins/src/index.ts | 1 + .../src/panels/NotebookPanel.tsx | 23 +++--- .../src/useConfigureRuff.ts | 23 ++++++ 7 files changed, 136 insertions(+), 77 deletions(-) create mode 100644 packages/dashboard-core-plugins/src/useConfigureRuff.ts diff --git a/packages/console/src/monaco/MonacoProviders.tsx b/packages/console/src/monaco/MonacoProviders.tsx index 7773dd8b5a..a2116cc89a 100644 --- a/packages/console/src/monaco/MonacoProviders.tsx +++ b/packages/console/src/monaco/MonacoProviders.tsx @@ -3,7 +3,6 @@ */ import { PureComponent } from 'react'; import * as monaco from 'monaco-editor'; -import throttle from 'lodash.throttle'; import Log from '@deephaven/log'; import type { dh } from '@deephaven/jsapi-types'; import init, { Workspace, type Diagnostic } from '@astral-sh/ruff-wasm-web'; @@ -64,7 +63,11 @@ class MonacoProviders extends PureComponent< MonacoProviders.ruffSettings = settings; // Ruff has not been initialized yet - if (MonacoProviders.ruffWorkspace == null) { + if ( + MonacoProviders.ruffWorkspace == null && + MonacoProviders.isRuffEnabled + ) { + MonacoProviders.initRuff(); return; } @@ -239,7 +242,7 @@ class MonacoProviders extends PureComponent< model: monaco.editor.ITextModel, range: monaco.Range ): monaco.languages.ProviderResult { - if (!MonacoProviders.ruffWorkspace) { + if (!MonacoProviders.isRuffEnabled || !MonacoProviders.ruffWorkspace) { return { actions: [], dispose: () => { @@ -397,7 +400,7 @@ class MonacoProviders extends PureComponent< } componentDidMount(): void { - const { language, session, model } = this.props; + const { language, session } = this.props; this.registeredCompletionProvider = monaco.languages.registerCompletionItemProvider(language, { @@ -421,23 +424,6 @@ class MonacoProviders extends PureComponent< } ); } - - if (language === 'python') { - if (MonacoProviders.ruffWorkspace == null) { - MonacoProviders.initRuff(); // This will also lint all open editors - } else { - MonacoProviders.lintPython(model); - } - - const throttledLint = throttle( - (m: monaco.editor.ITextModel) => MonacoProviders.lintPython(m), - 250 - ); - - model.onDidChangeContent(() => { - throttledLint(model); - }); - } } componentWillUnmount(): void { diff --git a/packages/console/src/monaco/MonacoUtils.ts b/packages/console/src/monaco/MonacoUtils.ts index b2477f81e2..3ee5b00d3a 100644 --- a/packages/console/src/monaco/MonacoUtils.ts +++ b/packages/console/src/monaco/MonacoUtils.ts @@ -1,5 +1,6 @@ /* eslint-disable @typescript-eslint/ban-ts-comment */ import { nanoid } from 'nanoid'; +import throttle from 'lodash.throttle'; /** * Exports a function for initializing monaco with the deephaven theme/config */ @@ -70,6 +71,24 @@ class MonacoUtils { }); }); + monaco.editor.onDidCreateModel(model => { + // Lint Python models on creation and on change + if (model.getLanguageId() === 'python') { + if (MonacoProviders.ruffWorkspace != null) { + MonacoProviders.lintPython(model); + } + + const throttledLint = throttle( + (m: monaco.editor.ITextModel) => MonacoProviders.lintPython(m), + 250 + ); + + model.onDidChangeContent(() => { + throttledLint(model); + }); + } + }); + MonacoUtils.removeConflictingKeybindings(); log.debug('Monaco initialized.'); @@ -547,6 +566,27 @@ class MonacoUtils { static isConsoleModel(model: monaco.editor.ITextModel): boolean { return model.uri.toString().startsWith(CONSOLE_URI_PREFIX); } + + /** + * Checks if the editor has the formatDocument action registered. + * @param editor The monaco editor to check + * @returns If the editor has a document formatter registered + */ + static canFormat(editor: monaco.editor.IStandaloneCodeEditor): boolean { + return ( + editor.getAction('editor.action.formatDocument')?.isSupported() === true + ); + } + + /** + * Runs the formatDocument action on the editor. + * @param editor The editor to format + */ + static async formatDocument( + editor: monaco.editor.IStandaloneCodeEditor + ): Promise { + await editor.getAction('editor.action.formatDocument')?.run(); + } } export default MonacoUtils; diff --git a/packages/console/src/monaco/RuffSettingsModal.tsx b/packages/console/src/monaco/RuffSettingsModal.tsx index 73670e71b0..d09e02f5ca 100644 --- a/packages/console/src/monaco/RuffSettingsModal.tsx +++ b/packages/console/src/monaco/RuffSettingsModal.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useRef, useState } from 'react'; +import React, { useCallback, useMemo, useRef, useState } from 'react'; import * as monaco from 'monaco-editor'; import { Workspace } from '@astral-sh/ruff-wasm-web'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; @@ -30,14 +30,10 @@ interface RuffSettingsModalProps { isOpen: boolean; onClose: () => void; onSave: (value: Record) => void; + readOnly?: boolean; + defaultSettings?: Record; } -const FORMATTED_DEFAULT_SETTINGS = JSON.stringify( - RUFF_DEFAULT_SETTINGS, - null, - 2 -); - const RUFF_SETTINGS_URI = monaco.Uri.parse( 'inmemory://dh-config/ruff-settings.json' ); @@ -71,11 +67,18 @@ export default function RuffSettingsModal({ isOpen, onClose, onSave, + readOnly = false, + defaultSettings = RUFF_DEFAULT_SETTINGS, }: RuffSettingsModalProps): React.ReactElement | null { const [isValid, setIsValid] = useState(false); const [isDefault, setIsDefault] = useState(false); const editorRef = useRef(); + const formattedDefaultSettings = useMemo( + () => JSON.stringify(defaultSettings, null, 2), + [defaultSettings] + ); + const { data: ruffVersion } = usePromiseFactory(getRuffVersion); const [model] = useState(() => @@ -103,20 +106,23 @@ export default function RuffSettingsModal({ const handleReset = useCallback((): void => { assertNotNull(model); - model.setValue(FORMATTED_DEFAULT_SETTINGS); - }, [model]); - - const validate = useCallback(val => { - try { - JSON.parse(val); - setIsValid(true); - } catch { - setIsValid(false); - } - setIsDefault( - editorRef.current?.getModel()?.getValue() === FORMATTED_DEFAULT_SETTINGS - ); - }, []); + model.setValue(formattedDefaultSettings); + }, [model, formattedDefaultSettings]); + + const validate = useCallback( + val => { + try { + JSON.parse(val); + setIsValid(true); + } catch { + setIsValid(false); + } + setIsDefault( + editorRef.current?.getModel()?.getValue() === formattedDefaultSettings + ); + }, + [formattedDefaultSettings] + ); const debouncedValidate = useDebouncedCallback(validate, 500, { leading: true, @@ -172,6 +178,7 @@ export default function RuffSettingsModal({ - - + {readOnly ? ( + + ) : ( + <> + + + + )} ); diff --git a/packages/dashboard-core-plugins/src/ConsolePlugin.tsx b/packages/dashboard-core-plugins/src/ConsolePlugin.tsx index eae350836a..863ee614ca 100644 --- a/packages/dashboard-core-plugins/src/ConsolePlugin.tsx +++ b/packages/dashboard-core-plugins/src/ConsolePlugin.tsx @@ -1,4 +1,4 @@ -import { MonacoProviders, type ScriptEditor } from '@deephaven/console'; +import { type ScriptEditor } from '@deephaven/console'; import { assertIsDashboardPluginProps, type DashboardPluginComponentProps, @@ -7,15 +7,13 @@ import { LayoutUtils, type PanelComponent, type PanelHydrateFunction, - useAppSelector, useListener, usePanelRegistration, } from '@deephaven/dashboard'; import { FileUtils } from '@deephaven/file-explorer'; import { type CloseOptions, isComponent } from '@deephaven/golden-layout'; import Log from '@deephaven/log'; -import { getNotebookSettings } from '@deephaven/redux'; -import { useCallback, useEffect, useRef, useState } from 'react'; +import { useCallback, useRef, useState } from 'react'; import { useDispatch } from 'react-redux'; import { nanoid } from 'nanoid'; import { ConsoleEvent, NotebookEvent } from './events'; @@ -27,6 +25,7 @@ import { NotebookPanel, } from './panels'; import { setDashboardConsoleSettings } from './redux'; +import useConfigureRuff from './useConfigureRuff'; const log = Log.module('ConsolePlugin'); @@ -76,15 +75,7 @@ export function ConsolePlugin( new Map() ); - const { python: { linter = {} } = {} } = useAppSelector(getNotebookSettings); - const { isEnabled: ruffEnabled = false, config: ruffConfig } = linter; - useEffect( - function setRuffSettings() { - MonacoProviders.isRuffEnabled = ruffEnabled; - MonacoProviders.setRuffSettings(ruffConfig); - }, - [ruffEnabled, ruffConfig] - ); + useConfigureRuff(); const dispatch = useDispatch(); diff --git a/packages/dashboard-core-plugins/src/index.ts b/packages/dashboard-core-plugins/src/index.ts index 476e909c01..475db25a53 100644 --- a/packages/dashboard-core-plugins/src/index.ts +++ b/packages/dashboard-core-plugins/src/index.ts @@ -22,6 +22,7 @@ export { default as ControlType } from './controls/ControlType'; export { default as LinkerUtils } from './linker/LinkerUtils'; export type { Link } from './linker/LinkerUtils'; export { default as ToolType } from './linker/ToolType'; +export * from './useConfigureRuff'; export * from './useLoadTablePlugin'; export * from './events'; diff --git a/packages/dashboard-core-plugins/src/panels/NotebookPanel.tsx b/packages/dashboard-core-plugins/src/panels/NotebookPanel.tsx index 732a0d221d..853287df95 100644 --- a/packages/dashboard-core-plugins/src/panels/NotebookPanel.tsx +++ b/packages/dashboard-core-plugins/src/panels/NotebookPanel.tsx @@ -15,7 +15,12 @@ import { type DropdownAction, LoadingOverlay, } from '@deephaven/components'; -import { ScriptEditor, ScriptEditorUtils, SHORTCUTS } from '@deephaven/console'; +import { + MonacoUtils, + ScriptEditor, + ScriptEditorUtils, + SHORTCUTS, +} from '@deephaven/console'; import { type FileStorage, FileUtils, @@ -1005,18 +1010,14 @@ class NotebookPanel extends Component { } canFormat(): boolean { - return ( - this.notebook?.editor - ?.getAction('editor.action.formatDocument') - ?.isSupported() === true - ); + return this.notebook?.editor != null + ? MonacoUtils.canFormat(this.notebook.editor) + : false; } async handleFormat(): Promise { - if (this.canFormat()) { - await this.notebook?.editor - ?.getAction('editor.action.formatDocument') - ?.run(); + if (this.notebook?.editor != null) { + await MonacoUtils.formatDocument(this.notebook.editor); } } @@ -1322,6 +1323,8 @@ class NotebookPanel extends Component { const portal = tab?.element.find('.lm_title_before').get(0); + console.log(this.notebook?.editor); + return ( <> {portal != null && diff --git a/packages/dashboard-core-plugins/src/useConfigureRuff.ts b/packages/dashboard-core-plugins/src/useConfigureRuff.ts new file mode 100644 index 0000000000..eba2ffa6b6 --- /dev/null +++ b/packages/dashboard-core-plugins/src/useConfigureRuff.ts @@ -0,0 +1,23 @@ +import { useEffect } from 'react'; +import { MonacoProviders } from '@deephaven/console'; +import { useAppSelector } from '@deephaven/dashboard'; +import { getNotebookSettings } from '@deephaven/redux'; + +/** + * Hook to configure Ruff settings in Monaco. + * The enabled status and settings are read from redux. + * Any changes to the redux values will be applied to the Monaco providers. + */ +export function useConfigureRuff(): void { + const { python: { linter = {} } = {} } = useAppSelector(getNotebookSettings); + const { isEnabled: ruffEnabled = false, config: ruffConfig } = linter; + useEffect( + function setRuffSettings() { + MonacoProviders.isRuffEnabled = ruffEnabled; + MonacoProviders.setRuffSettings(ruffConfig); // Also inits Ruff if needed + }, + [ruffEnabled, ruffConfig] + ); +} + +export default useConfigureRuff; From 3dc2001b9b83842d323e52d1dac63651dcb9f678 Mon Sep 17 00:00:00 2001 From: Matthew Runyon Date: Tue, 5 Nov 2024 20:52:57 -0600 Subject: [PATCH 2/4] Remove console log --- packages/dashboard-core-plugins/src/panels/NotebookPanel.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/dashboard-core-plugins/src/panels/NotebookPanel.tsx b/packages/dashboard-core-plugins/src/panels/NotebookPanel.tsx index 853287df95..f5101dbd9a 100644 --- a/packages/dashboard-core-plugins/src/panels/NotebookPanel.tsx +++ b/packages/dashboard-core-plugins/src/panels/NotebookPanel.tsx @@ -1323,8 +1323,6 @@ class NotebookPanel extends Component { const portal = tab?.element.find('.lm_title_before').get(0); - console.log(this.notebook?.editor); - return ( <> {portal != null && From d5b7f9d6e9f2288429a2a81d83bfd73920192f49 Mon Sep 17 00:00:00 2001 From: Matthew Runyon Date: Wed, 6 Nov 2024 09:31:15 -0600 Subject: [PATCH 3/4] Fix tests --- __mocks__/@astral-sh/ruff-wasm-web.js | 3 +++ jest.config.base.cjs | 4 ++++ 2 files changed, 7 insertions(+) create mode 100644 __mocks__/@astral-sh/ruff-wasm-web.js diff --git a/__mocks__/@astral-sh/ruff-wasm-web.js b/__mocks__/@astral-sh/ruff-wasm-web.js new file mode 100644 index 0000000000..8124c79ba3 --- /dev/null +++ b/__mocks__/@astral-sh/ruff-wasm-web.js @@ -0,0 +1,3 @@ +const init = jest.fn(); +export const Workspace = jest.fn(); +export default init; diff --git a/jest.config.base.cjs b/jest.config.base.cjs index 1dd7d695f4..07d90e1731 100644 --- a/jest.config.base.cjs +++ b/jest.config.base.cjs @@ -65,6 +65,10 @@ module.exports = { ), // Handle monaco worker files '\\.worker.*$': 'identity-obj-proxy', + '^@astral-sh/ruff-wasm-web$': path.join( + __dirname, + './__mocks__/@astral-sh/ruff-wasm-web.js' + ), // Handle pouchdb modules '^pouchdb-browser$': path.join( __dirname, From 3f99af5f0bd71c2d5a77629bea712ec155969b4e Mon Sep 17 00:00:00 2001 From: Matthew Runyon Date: Thu, 7 Nov 2024 16:16:23 -0600 Subject: [PATCH 4/4] Add contextual help for format on save --- .../src/settings/EditorSectionContent.tsx | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/packages/code-studio/src/settings/EditorSectionContent.tsx b/packages/code-studio/src/settings/EditorSectionContent.tsx index eab21ff827..8de19ff214 100644 --- a/packages/code-studio/src/settings/EditorSectionContent.tsx +++ b/packages/code-studio/src/settings/EditorSectionContent.tsx @@ -1,6 +1,14 @@ import React, { useCallback, useState } from 'react'; import { useDispatch } from 'react-redux'; -import { Switch, ActionButton, Icon, Text } from '@deephaven/components'; +import { + Switch, + ActionButton, + Icon, + Text, + ContextualHelp, + Heading, + Content, +} from '@deephaven/components'; import { useAppSelector } from '@deephaven/dashboard'; import { getNotebookSettings, updateNotebookSettings } from '@deephaven/redux'; import { vsSettings } from '@deephaven/icons'; @@ -95,10 +103,23 @@ export function EditorSectionContent(): JSX.Element { Enable Minimap -
- +
+ Format on Save + + Format on Save + + + The Ruff settings control formatting options. Notebooks can be + formatted manually using the right-click context menu. + + +