Skip to content

Commit

Permalink
feat: Ruff Configuration Settings (#2215)
Browse files Browse the repository at this point in the history
Fixes #1255 

I need to test this doesn't crash if using groovy. Not sure if the ruff
version is available without initializing ruff first or what happens if
it's unavailable.

- Added global setting for minimap, format on save, formatter, and
formatting settings
- Added format on save option and format button to notebook overflow
menu
- Format on save triggers on both shortcut and pressing the save button
- Config editor has JSON schema included
- Config editor cannot be closed by clicking outside the modal, but can
be closed w/ escape key

Something we should add maybe as part of this PR or a final follow-up to
this feature branch is disabling certain/all linting/formatting for the
console. One way we might do this is by registering the console w/ a
specific URI or URI scheme so we can check it when checking if we should
lint. Or include it as a prop to the `MonacoProviders` component. This
would require some changes to the current setup of setting/using the
config on `MonacoProviders` static members/methods

(This might not actually be breaking, it's more just notes for
implementing in DHE)
BREAKING CHANGE:
The app should call `MonacoUtils.init` with a `getWorker` function that
uses the JSON worker in addition to the general fallback worker when
adding support for configuring ruff.
  • Loading branch information
mattrunyon committed Sep 20, 2024
1 parent aaf4325 commit 3144edb
Show file tree
Hide file tree
Showing 37 changed files with 4,558 additions and 1,142 deletions.
1 change: 0 additions & 1 deletion babel.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ module.exports = api => {
!isTest ? /\.test.(tsx?|jsx?)$/ : false,
!isTest ? '**/__mocks__/*' : false,
'**/*.scss',
'**/*.d.ts',
].filter(Boolean),
};
};
1 change: 1 addition & 0 deletions jest.config.base.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const nodeModulesToTransform = [
'web-namespaces',
'hastscript',
'nanoid',
'@astral-sh/ruff-wasm-web',
];

module.exports = {
Expand Down
57 changes: 45 additions & 12 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
"@types/lodash.clamp": "^4.0.6",
"@types/lodash.debounce": "^4.0.6",
"@types/lodash.flatten": "^4.4.0",
"@types/lodash.merge": "^4.6.9",
"@types/lodash.set": "^4.3.7",
"@types/lodash.throttle": "^4.1.1",
"@types/memoizee": "^0.4.5",
Expand Down
1 change: 1 addition & 0 deletions packages/app-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
"@paciolan/remote-module-loader": "^3.0.2",
"classnames": "^2.5.1",
"lodash.debounce": "^4.0.8",
"lodash.merge": "^4.6.2",
"lodash.throttle": "^4.1.1"
},
"peerDependencies": {
Expand Down
79 changes: 55 additions & 24 deletions packages/app-utils/src/storage/LocalWorkspaceStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
DecimalColumnFormatter,
IntegerColumnFormatter,
} from '@deephaven/jsapi-utils';
import merge from 'lodash.merge';
import UserLayoutUtils from './UserLayoutUtils';
import LayoutStorage from './LayoutStorage';

Expand All @@ -36,6 +37,26 @@ export class LocalWorkspaceStorage implements WorkspaceStorage {
return undefined;
}

static getJSONServerConfig(
serverConfigValues: Map<string, string> | undefined,
key: string
): Record<string, unknown> | undefined {
const value = serverConfigValues?.get(key);

if (value == null) {
return undefined;
}

try {
return JSON.parse(value);
} catch (e) {
log.error(
`Unable to parse JSON server config for key ${key}. Value: ${value}`
);
return undefined;
}
}

static makeDefaultWorkspaceSettings(
serverConfigValues: ServerConfigValues
): WorkspaceSettings {
Expand All @@ -57,13 +78,20 @@ export class LocalWorkspaceStorage implements WorkspaceStorage {
showEmptyStrings: true,
showNullStrings: true,
showExtraGroupColumn: true,
defaultNotebookSettings: {
notebookSettings: {
isMinimapEnabled: false,
formatOnSave: false,
python: {
linter: {
isEnabled: true,
// Omit default config so default settings are used if the user never changes them
},
},
},
webgl: true,
webglEditable: true,
gridDensity: 'regular' as const,
};
gridDensity: 'regular',
} satisfies WorkspaceSettings;
const serverSettings = {
defaultDateTimeFormat: serverConfigValues?.get('dateTimeFormat'),
formatter: [],
Expand Down Expand Up @@ -108,15 +136,28 @@ export class LocalWorkspaceStorage implements WorkspaceStorage {
serverConfigValues,
'showExtraGroupColumn'
),
defaultNotebookSettings:
serverConfigValues?.get('isMinimapEnabled') !== undefined
? {
isMinimapEnabled: LocalWorkspaceStorage.getBooleanServerConfig(
serverConfigValues,
'isMinimapEnabled'
) as boolean,
}
: undefined,
notebookSettings: {
isMinimapEnabled: LocalWorkspaceStorage.getBooleanServerConfig(
serverConfigValues,
'web.user.notebookSettings.isMinimapEnabled'
),
formatOnSave: LocalWorkspaceStorage.getBooleanServerConfig(
serverConfigValues,
'web.user.notebookSettings.formatOnSave'
),
python: {
linter: {
isEnabled: LocalWorkspaceStorage.getBooleanServerConfig(
serverConfigValues,
'web.user.notebookSettings.python.linter.isEnabled'
),
config: LocalWorkspaceStorage.getJSONServerConfig(
serverConfigValues,
'web.user.notebookSettings.python.linter.config'
),
},
},
},
webgl: LocalWorkspaceStorage.getBooleanServerConfig(
serverConfigValues,
'web.webgl'
Expand All @@ -125,19 +166,9 @@ export class LocalWorkspaceStorage implements WorkspaceStorage {
serverConfigValues,
'web.webgl.editable'
),
};
} satisfies Partial<WorkspaceSettings>;

const keys = Object.keys(serverSettings) as Array<
keyof typeof serverSettings
>;
for (let i = 0; i < keys.length; i += 1) {
const key = keys[i];
if (serverSettings[key] !== undefined) {
// @ts-expect-error override default for defined server settings
settings[key] = serverSettings[key];
}
}
return settings;
return merge({}, settings, serverSettings);
}

static async makeWorkspaceData(
Expand Down
2 changes: 1 addition & 1 deletion packages/code-studio/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"lodash.throttle": "^4.1.1",
"memoize-one": "^5.1.1",
"memoizee": "^0.4.15",
"monaco-editor": "^0.41.0",
"monaco-editor": "^0.43.0",
"nanoid": "^5.0.7",
"pouchdb-browser": "^7.2.2",
"pouchdb-find": "^7.2.2",
Expand Down
10 changes: 9 additions & 1 deletion packages/code-studio/src/AppRoot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React from 'react';
import { MonacoUtils } from '@deephaven/console';
import { DownloadServiceWorkerUtils } from '@deephaven/iris-grid';
import MonacoWorker from 'monaco-editor/esm/vs/editor/editor.worker?worker';
import MonacoJsonWorker from 'monaco-editor/esm/vs/language/json/json.worker?worker';
import AppRouter from './main/AppRouter';

// load addional css for playwright docker tests
Expand All @@ -16,7 +17,14 @@ export function AppRoot(): JSX.Element {
window.location.href
)
);
MonacoUtils.init({ getWorker: () => new MonacoWorker() });
MonacoUtils.init({
getWorker: (id: string, label: string) => {
if (label === 'json') {
return new MonacoJsonWorker();
}
return new MonacoWorker();
},
});

// disable annoying dnd-react warnings
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
Expand Down
Loading

0 comments on commit 3144edb

Please sign in to comment.