Skip to content

Commit

Permalink
feat: Ruff updates for DHE support (#2280)
Browse files Browse the repository at this point in the history
Support for DH-17923 and also fixes some other edge cases with Ruff.

- Adds a hook for configuring Ruff and keeping it in sync w/ redux
settings.
- Fixes a bug where if Ruff was enabled, but there was no user session
then quick fixes were shown but linting underlines were not. Changed so
Python is always linted if opened since it doesn't require a session to
lint.
- Allow passing a default config to the Ruff config editor since DHE can
set a custom config as the default via server props
- Added readOnly mode to the Ruff config editor
- Moved some formatting logic into MonacoUtils since DHE implements its
own notebook panel. Eventually we should just consume the DHC
NotebookPanel in DHE with extensions for PQs and other DHE options
  • Loading branch information
mattrunyon authored Nov 12, 2024
1 parent 8ccab6d commit a35625e
Show file tree
Hide file tree
Showing 10 changed files with 165 additions and 80 deletions.
3 changes: 3 additions & 0 deletions __mocks__/@astral-sh/ruff-wasm-web.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const init = jest.fn();
export const Workspace = jest.fn();
export default init;
4 changes: 4 additions & 0 deletions jest.config.base.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
27 changes: 24 additions & 3 deletions packages/code-studio/src/settings/EditorSectionContent.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -95,10 +103,23 @@ export function EditorSectionContent(): JSX.Element {
Enable Minimap
</Switch>
</div>
<div className="form-row pl-1">
<Switch isSelected={formatOnSave} onChange={handleFormatOnSaveChange}>
<div className="form-row align-items-center pl-1">
<Switch
isSelected={formatOnSave}
onChange={handleFormatOnSaveChange}
margin={0}
>
Format on Save
</Switch>
<ContextualHelp variant="info">
<Heading>Format on Save</Heading>
<Content>
<Text>
The Ruff settings control formatting options. Notebooks can be
formatted manually using the right-click context menu.
</Text>
</Content>
</ContextualHelp>
</div>
<div className="form-row pl-1">
<Switch
Expand Down
28 changes: 7 additions & 21 deletions packages/console/src/monaco/MonacoProviders.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -239,7 +242,7 @@ class MonacoProviders extends PureComponent<
model: monaco.editor.ITextModel,
range: monaco.Range
): monaco.languages.ProviderResult<monaco.languages.CodeActionList> {
if (!MonacoProviders.ruffWorkspace) {
if (!MonacoProviders.isRuffEnabled || !MonacoProviders.ruffWorkspace) {
return {
actions: [],
dispose: () => {
Expand Down Expand Up @@ -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, {
Expand All @@ -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 {
Expand Down
40 changes: 40 additions & 0 deletions packages/console/src/monaco/MonacoUtils.ts
Original file line number Diff line number Diff line change
@@ -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
*/
Expand Down Expand Up @@ -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.');
Expand Down Expand Up @@ -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<void> {
await editor.getAction('editor.action.formatDocument')?.run();
}
}

export default MonacoUtils;
81 changes: 48 additions & 33 deletions packages/console/src/monaco/RuffSettingsModal.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -30,14 +30,10 @@ interface RuffSettingsModalProps {
isOpen: boolean;
onClose: () => void;
onSave: (value: Record<string, unknown>) => void;
readOnly?: boolean;
defaultSettings?: Record<string, unknown>;
}

const FORMATTED_DEFAULT_SETTINGS = JSON.stringify(
RUFF_DEFAULT_SETTINGS,
null,
2
);

const RUFF_SETTINGS_URI = monaco.Uri.parse(
'inmemory://dh-config/ruff-settings.json'
);
Expand Down Expand Up @@ -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<monaco.editor.IStandaloneCodeEditor>();

const formattedDefaultSettings = useMemo(
() => JSON.stringify(defaultSettings, null, 2),
[defaultSettings]
);

const { data: ruffVersion } = usePromiseFactory(getRuffVersion);

const [model] = useState(() =>
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -172,6 +178,7 @@ export default function RuffSettingsModal({
<Editor
onEditorInitialized={onEditorInitialized}
settings={{
readOnly,
value: text,
language: 'json',
folding: true,
Expand All @@ -183,18 +190,26 @@ export default function RuffSettingsModal({
/>
</ModalBody>
<ModalFooter>
<Button kind="secondary" data-dismiss="modal" onClick={handleClose}>
Cancel
</Button>
<Button
kind="primary"
data-dismiss="modal"
tooltip={!isValid ? 'Cannot save invalid JSON' : undefined}
disabled={!isValid}
onClick={handleSave}
>
Save
</Button>
{readOnly ? (
<Button kind="secondary" data-dismiss="modal" onClick={handleClose}>
Close
</Button>
) : (
<>
<Button kind="secondary" data-dismiss="modal" onClick={handleClose}>
Cancel
</Button>
<Button
kind="primary"
data-dismiss="modal"
tooltip={!isValid ? 'Cannot save invalid JSON' : undefined}
disabled={!isValid}
onClick={handleSave}
>
Save
</Button>
</>
)}
</ModalFooter>
</Modal>
);
Expand Down
17 changes: 4 additions & 13 deletions packages/dashboard-core-plugins/src/ConsolePlugin.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { MonacoProviders, type ScriptEditor } from '@deephaven/console';
import { type ScriptEditor } from '@deephaven/console';
import {
assertIsDashboardPluginProps,
type DashboardPluginComponentProps,
Expand All @@ -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';
Expand All @@ -27,6 +25,7 @@ import {
NotebookPanel,
} from './panels';
import { setDashboardConsoleSettings } from './redux';
import useConfigureRuff from './useConfigureRuff';

const log = Log.module('ConsolePlugin');

Expand Down Expand Up @@ -76,15 +75,7 @@ export function ConsolePlugin(
new Map<string, string>()
);

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();

Expand Down
1 change: 1 addition & 0 deletions packages/dashboard-core-plugins/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Loading

0 comments on commit a35625e

Please sign in to comment.