[v2/Windows]Set default extension for save file dialog #3205
Conversation
WalkthroughThe update addresses a functional issue related to file handling in a desktop application framework on Windows. Specifically, it introduces a fix for the Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| if len(options.Filters) > 0 { | ||
| config.DefaultExtension = strings.TrimPrefix(strings.Split(options.Filters[0].Pattern, ";")[0], "*") | ||
| } |
There was a problem hiding this comment.
The logic to set the default file extension assumes that the first filter's pattern contains the desired default extension and that it is separated by a semicolon. This may not always be the case, and the code does not handle potential edge cases where the pattern might not include a semicolon or might include multiple extensions. Consider adding more robust parsing to handle different filter formats.
- config.DefaultExtension = strings.TrimPrefix(strings.Split(options.Filters[0].Pattern, ";")[0], "*")
+ // Proposed change: Add a function to parse and return the default extension
+ config.DefaultExtension = parseDefaultExtension(options.Filters)Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| if len(options.Filters) > 0 { | |
| config.DefaultExtension = strings.TrimPrefix(strings.Split(options.Filters[0].Pattern, ";")[0], "*") | |
| } | |
| if len(options.Filters) > 0 { | |
| // Proposed change: Add a function to parse and return the default extension | |
| config.DefaultExtension = parseDefaultExtension(options.Filters) | |
| } |
There was a problem hiding this comment.
Splitting on a non existent separator is fine if you're only interested in the first element
There was a problem hiding this comment.
@leaanthony Thank you for the clarification. Understood, if the current implementation meets the intended use case and handles the scenarios as expected, there's no need for additional changes.
There was a problem hiding this comment.
Splitting on a non existent separator is fine if you're only interested in the first element
The reason I split by semicolon first is to handle patterns with multiple extensions, such as
{
DisplayName: "Image Files (*.jpg, *.png)",
Pattern: "*.jpg;*.png",
}Just take the first extension, i.e. .jpg
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- website/src/pages/changelog.mdx (1 hunks)
Additional comments: 1
website/src/pages/changelog.mdx (1)
- 29-29: The changelog entry correctly reflects the fix for the save file dialog issue on Windows.
|
if I change my filter，Will the default suffix change based on the filter I select? |
If the user selects a different file filter, the default extension will be automatically updated to match the new file filter. |
|
Thanks @almas1992 🙏 |
Description
According to the comment, the default extension should be set when filters are set, so that the filename is automatically appended with the extension.
Fixes #3202
Type of change
Please delete options that are not 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.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.mdxwith details of this PRSummary by CodeRabbit