fix: Change ruff errors to warnings and fix config saving#2246
fix: Change ruff errors to warnings and fix config saving#2246mattrunyon merged 1 commit intodeephaven:mainfrom
Conversation
| linter: { | ||
| ...ruffSettings, | ||
| config: | ||
| JSON.stringify(newValue) === |
There was a problem hiding this comment.
Is the argument mismatch in the JSON.stringify calls intentional?
There was a problem hiding this comment.
Yes. I want to check if the value was reset to default here, but the objects won't be the same. The other option was to do this check in the settings modal before onSave is called and then call with null or undefined to indicate the default.
There was a problem hiding this comment.
Oh, I misread this. I thought your were passing in undefined, 2 to one of the JSON.stringify calls
| ); | ||
|
|
||
| const [val, setVal] = useState(JSON.stringify(ruffConfig, null, 2)); | ||
| const val = JSON.stringify(ruffConfig, null, 2); |
There was a problem hiding this comment.
I don't think it matters. The object isn't that big and there's still some overhead to do the string comparison for memoization. Also the component shouldn't re-render often.
bmingles
left a comment
bmingles
left a comment
There was a problem hiding this comment.
Looks good overall. Left 2 suggestion / questions
2 participants
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.