-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[V3] Windows: fix(application): handle error and type assertion in save file dialog #4284
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
base: v3-alpha
Are you sure you want to change the base?
Conversation
WalkthroughThe changes introduce improved error handling for the Windows SaveFileDialog, specifically addressing cases where the dialog is cancelled or closed. The modifications ensure that errors are correctly propagated and handled, preventing application crashes. Documentation is updated to reflect the fix for the panic issue when cancelling the dialog. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Application
participant SaveFileDialog
participant OS
User->>Application: Initiate SaveFileDialog
Application->>SaveFileDialog: show()
SaveFileDialog->>OS: showCfdDialog()
OS-->>SaveFileDialog: Return result or error
alt Error or Cancelled
SaveFileDialog->>Application: Return closed channel, error
else Success
SaveFileDialog->>Application: Send result (string) on channel
end
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related PRs
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
website/src/pages/changelog.mdx (1)
19-19
: Good documentation of the fix in the changelog.The changelog entry clearly describes the issue that was fixed and references the PR number. This helps users and developers understand what was addressed in this release.
Note: "Windows" should be capitalized as it refers to the operating system.
- Fixed panic when closing or cancelling a `SaveFileDialog` on windows. Fixed in [PR](https://github.com/wailsapp/wails/pull/4284) + Fixed panic when closing or cancelling a `SaveFileDialog` on Windows. Fixed in [PR](https://github.com/wailsapp/wails/pull/4284)🧰 Tools
🪛 LanguageTool
[grammar] ~19-~19: “Windows” (operating system by Microsoft) is a proper noun and needs to be capitalized.
Context: ...ing or cancelling aSaveFileDialog
on windows. Fixed in [PR](https://github.com/wails...(A_WINDOWS)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (3)
v3/internal/go-common-file-dialog/cfd/vtblCommonFunc.go
(2 hunks)v3/pkg/application/dialogs_windows.go
(1 hunks)website/src/pages/changelog.mdx
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
website/src/pages/changelog.mdx
[grammar] ~19-~19: “Windows” (operating system by Microsoft) is a proper noun and needs to be capitalized.
Context: ...ing or cancelling a SaveFileDialog
on windows. Fixed in [PR](https://github.com/wails...
(A_WINDOWS)
🔇 Additional comments (3)
v3/internal/go-common-file-dialog/cfd/vtblCommonFunc.go (1)
171-173
: Improved error handling with specific error type.This change enhances error handling by returning a specific
ErrorCancelled
error rather than a generic error message. This is a better practice as it allows calling code to check for specific error types rather than parsing error strings.v3/pkg/application/dialogs_windows.go (2)
199-202
: Fixed potential resource leak by properly handling errors.This is a critical fix that properly handles errors from
showCfdDialog
by closing the channel and returning early. Previously, an error could have resulted in a resource leak or incorrect behavior because the channel wouldn't be properly closed.
205-208
: Added type safety to prevent panic on invalid type assertion.This change adds proper type assertion with a check, preventing a potential panic if the result is not of the expected type. This is good defensive programming that makes the code more robust against unexpected scenarios.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/src/content/docs/changelog.mdx (1)
131-131
: Capitalize “Windows” and align entry style.The new fix entry currently reads “on windows.” Windows is a proper noun and should be capitalized. Also for consistency with other OS‐specific entries, consider adding the ⊞ badge and using the
[#4284]
PR reference style:- - Fixed panic when closing or cancelling a `SaveFileDialog` on windows. Fixed in [PR](https://github.com/wailsapp/wails/pull/4284) by @hkhere + - ⊞ Fixed panic when closing or cancelling a `SaveFileDialog` on Windows. Fixed in [#4284](https://github.com/wailsapp/wails/pull/4284) by @hkhere🧰 Tools
🪛 LanguageTool
[grammar] ~131-~131: “Windows” (operating system by Microsoft) is a proper noun and needs to be capitalized.
Context: ...ing or cancelling aSaveFileDialog
on windows. Fixed in [PR](https://github.com/wails...(A_WINDOWS)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/src/content/docs/changelog.mdx
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/src/content/docs/changelog.mdx
[grammar] ~131-~131: “Windows” (operating system by Microsoft) is a proper noun and needs to be capitalized.
Context: ...ing or cancelling a SaveFileDialog
on windows. Fixed in [PR](https://github.com/wails...
(A_WINDOWS)
|
Description
This fixes a failure when closing or cancelling a
SaveFileDialog
on windows.Ensure the save file dialog properly handles errors and safely asserts the result type to avoid potential runtime panics. Additionally, improve error handling in the file dialog result retrieval to return a specific error when the operation is cancelled.
Fixes #4279
Type of change
Please select the option that is relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration using
wails doctor
.If you checked Linux, please specify the distro and version.
Test Configuration
Please paste the output of
wails doctor
. If you are unable to run this command, please describe your environment in as much detail as possible.Checklist:
website/src/pages/changelog.mdx
with details of this PRSummary by CodeRabbit
Bug Fixes
Documentation