fix: Change ruff errors to warnings and fix config saving (#2246)

Changed all except syntax errors to warning underlines instead of error underline since they are not actual errors. Also changed redux to not save if the config matches the default. This should fix an edge case where a user opens the settings editor just to see what the config is, then hits "Save". Previously, this would save the current defaults as their settings and then if the defaults changed they would not get updated settings. This way, if a user was on defaults and the default changes they will get the new default. Also added a little bit of debug logging for Ruff.
deephaven · Oct 4, 2024 · 6ae25a2 · 6ae25a2
1 parent 92b133b
commit 6ae25a2
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 9 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/packages/code-studio/src/settings/EditorSectionContent.tsx b/packages/code-studio/src/settings/EditorSectionContent.tsx
@@ -51,15 +51,24 @@ export function EditorSectionContent(): JSX.Element {
     (newValue: Record<string, unknown>) => {
       dispatch(
         updateNotebookSettings({
-          python: { linter: { ...ruffSettings, config: newValue } },
+          python: {
+            linter: {
+              ...ruffSettings,
+              config:
+                JSON.stringify(newValue) ===
+                JSON.stringify(RUFF_DEFAULT_SETTINGS)
+                  ? undefined
+                  : newValue,
+            },
+          },
         })
       );
       MonacoProviders.setRuffSettings(newValue);
     },
     [dispatch, ruffSettings]
   );
 
-  const [val, setVal] = useState(JSON.stringify(ruffConfig, null, 2));
+  const val = JSON.stringify(ruffConfig, null, 2);
 
   const [isRuffSettingsOpen, setIsRuffSettingsOpen] = useState(false);
 
@@ -70,7 +79,6 @@ export function EditorSectionContent(): JSX.Element {
   const handleRuffSettingsSave = useCallback(
     (v: Record<string, unknown>) => {
       handleRuffConfigChange(v);
-      setVal(JSON.stringify(v, null, 2));
       handleRuffSettingsClose();
     },
     [handleRuffConfigChange, handleRuffSettingsClose]

diff --git a/packages/console/src/monaco/MonacoProviders.tsx b/packages/console/src/monaco/MonacoProviders.tsx
@@ -108,21 +108,25 @@ class MonacoProviders extends PureComponent<
       return;
     }
 
+    const diagnostics = MonacoProviders.getDiagnostics(model);
+    log.debug(`Linting Python document: ${model.uri.toString()}`, diagnostics);
+
     monaco.editor.setModelMarkers(
       model,
       'ruff',
-      MonacoProviders.getDiagnostics(model).map((d: Diagnostic) => {
-        // Unused variable or import. Mark as warning and unnecessary to fade the text
+      diagnostics.map((d: Diagnostic) => {
+        // Unused variable or import. Mark as unnecessary to fade the text
         const isUnnecessary = d.code === 'F401' || d.code === 'F841';
+        const isSyntaxError = d.code == null; // SyntaxError has no error code
         return {
           startLineNumber: d.location.row,
           startColumn: d.location.column,
           endLineNumber: d.end_location.row,
           endColumn: d.end_location.column,
-          message: d.code != null ? `${d.code}: ${d.message}` : d.message, // SyntaxError has no code
-          severity: isUnnecessary
-            ? monaco.MarkerSeverity.Warning
-            : monaco.MarkerSeverity.Error,
+          message: isSyntaxError ? d.message : `${d.code}: ${d.message}`,
+          severity: isSyntaxError
+            ? monaco.MarkerSeverity.Error
+            : monaco.MarkerSeverity.Warning,
           tags: isUnnecessary ? [monaco.MarkerTag.Unnecessary] : [],
         };
       })
@@ -374,6 +378,8 @@ class MonacoProviders extends PureComponent<
       return;
     }
 
+    log.debug(`Formatting Python document: ${model.uri.toString}`);
+
     return [
       {
         range: model.getFullModelRange(),