From 6ae25a258ff4868d74e01040bbdf959bc7dd5586 Mon Sep 17 00:00:00 2001
From: Matthew Runyon <matthewrunyon@deephaven.io>
Date: Fri, 4 Oct 2024 10:17:41 -0500
Subject: [PATCH] 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.
---
 package-lock.json                              |  2 ++
 .../src/settings/EditorSectionContent.tsx      | 14 +++++++++++---
 .../console/src/monaco/MonacoProviders.tsx     | 18 ++++++++++++------
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/package-lock.json b/package-lock.json
index 55cdb195f8..1ebccc333b 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -29625,6 +29625,7 @@
         "@deephaven/icons": "file:../icons",
         "@deephaven/jsapi-bootstrap": "file:../jsapi-bootstrap",
         "@deephaven/jsapi-types": "^1.0.0-dev0.34.0",
+        "@deephaven/jsapi-utils": "file:../jsapi-utils",
         "@deephaven/log": "file:../log",
         "@deephaven/react-hooks": "file:../react-hooks",
         "@deephaven/storage": "file:../storage",
@@ -32090,6 +32091,7 @@
         "@deephaven/jsapi-bootstrap": "file:../jsapi-bootstrap",
         "@deephaven/jsapi-shim": "file:../jsapi-shim",
         "@deephaven/jsapi-types": "^1.0.0-dev0.34.0",
+        "@deephaven/jsapi-utils": "file:../jsapi-utils",
         "@deephaven/log": "file:../log",
         "@deephaven/mocks": "file:../mocks",
         "@deephaven/react-hooks": "file:../react-hooks",
diff --git a/packages/code-studio/src/settings/EditorSectionContent.tsx b/packages/code-studio/src/settings/EditorSectionContent.tsx
index c2bd70a5a2..eab21ff827 100644
--- a/packages/code-studio/src/settings/EditorSectionContent.tsx
+++ b/packages/code-studio/src/settings/EditorSectionContent.tsx
@@ -51,7 +51,16 @@ 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);
@@ -59,7 +68,7 @@ export function EditorSectionContent(): JSX.Element {
     [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
index 5f3f6ae5d7..7773dd8b5a 100644
--- 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(),