-
Notifications
You must be signed in to change notification settings - Fork 3
Cleaning up #358
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cleaning up #358
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR aims to clean up the codebase by replacing explicit comparisons with undefined/null/empty strings with more concise truthy checks. However, the changes introduce multiple critical bugs where numeric zero values and empty strings are incorrectly treated as falsy when they are valid values in the application's logic.
Changes:
- Replaced explicit
=== undefined,!== undefined,=== null,!== null, and=== ''comparisons with truthy/falsy checks (!value,value, etc.) - Updated version numbers in package.json files
- Refactored some ternary operator patterns
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| src/renderer/src/libopencor/locUiJsonApi.ts | Simplified validation checks for UI JSON properties, introducing several bugs with 'in' operator usage and numeric zero handling |
| src/renderer/src/libopencor/locFileApi.ts | Simplified undefined checks for file contents and UI JSON |
| src/renderer/src/libopencor/locApi.ts | Simplified check for window.locApi |
| src/renderer/src/components/widgets/InputWidget.vue | Simplified checks for possible values, minimum/maximum values, and slider values - introduces critical bugs with numeric zero |
| src/renderer/src/components/widgets/GraphPanelWidget.vue | Simplified margin and data checks - introduces critical bugs with numeric zero margins |
| src/renderer/src/components/views/SimulationExperimentView.vue | Simplified array length and data checks - introduces bugs with index 0 and empty strings |
| src/renderer/src/components/views/IssuesView.vue | Simplified width/height checks - introduces critical bug with zero dimensions |
| src/renderer/src/components/dialogs/OpenRemoteDialog.vue | Simplified URL validation |
| src/renderer/src/components/dialogs/BaseDialog.vue | Simplified null checks for DOM elements |
| src/renderer/src/components/dialogs/AboutDialog.vue | Simplified electronApi check |
| src/renderer/src/components/OpenCOR.vue | Simplified various undefined/null checks and refactored comparison logic |
| src/renderer/src/components/MainMenu.vue | Simplified checks for menu items |
| src/renderer/src/components/ContentsComponent.vue | Simplified array length and tab checks |
| src/renderer/src/common/vueCommon.ts | Simplified string checks and used nullish coalescing operator |
| src/renderer/src/common/settings.ts | Simplified electronApi checks |
| src/renderer/src/common/rendererServer.ts | Simplified server null checks |
| src/renderer/src/common/locCommon.ts | Simplified name validation and electronApi checks |
| src/renderer/src/common/gitHubIntegration.ts | Simplified entry and token validation |
| src/renderer/src/common/firebaseConfig.ts | Inverted ternary logic for missing keys check |
| src/renderer/src/common/common.ts | Simplified time formatting checks |
| src/renderer/package.json | Version bump |
| src/main/index.ts | Simplified null checks and used nullish coalescing operator |
| src/main/MainWindow.ts | Simplified array filtering and length checks - introduces bug with empty string filtering |
| src/main/MainMenu.ts | Simplified menu item checks |
| src/extern/corsProxy.js | Simplified URL parameter check |
| package.json | Version bump |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/renderer/src/libopencor/locUiJsonApi.ts:429
- The refactored condition changes validation behavior. The original code checked
input.visible !== undefined && input.visible === '', which only triggered for explicitly empty strings. The new code'visible' in input && !input.visibletriggers for any falsy value including empty string''andundefined. Sincevisibleis typed asstring | undefined, this means settingvisible: undefinedexplicitly (e.g., in an object literal) would now trigger the error, whereas it wouldn't before. Verify this is the intended behavior.
if ('visible' in input && !input.visible) {
res.push({
type: EIssueType.WARNING,
description: 'UI JSON: an input visible must not be empty.'
});
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.