Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[v2/Windows]Set default extension for save file dialog #3205

Merged
merged 3 commits into from
Jan 17, 2024
Merged

[v2/Windows]Set default extension for save file dialog #3205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fa982a6
Set default extension for save file dialog
almas1992 Jan 16, 2024
1fd0d9f
add change log
almas1992 Jan 16, 2024
f6b701a
Merge branch 'master' into master
leaanthony Jan 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions v2/internal/frontend/desktop/windows/dialog.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ func (f *Frontend) SaveFileDialog(options frontend.SaveDialogOptions) (string, e
Folder: defaultFolder,
}

if len(options.Filters) > 0 {
config.DefaultExtension = strings.TrimPrefix(strings.Split(options.Filters[0].Pattern, ";")[0], "*")
}
Comment on lines +124 to +126
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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)
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Splitting on a non existent separator is fine if you're only interested in the first element

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!


result, err := f.showCfdDialog(
func() (cfd.Dialog, error) {
return cfd.NewSaveFileDialog(config)
Expand Down
1 change: 1 addition & 0 deletions website/src/pages/changelog.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Docs for IsZoomControlEnabled and ZoomFactor. Fixed by @leaanthony in [PR](https://github.com/wailsapp/wails/pull/3137)
- Fixed `-compiler` flag for `wails build`, `wails dev` and `wails generate module`. Fixed in [PR](https://github.com/wailsapp/wails/pull/3121) by [@xtrafrancyz](https://github.com/xtrafrancyz)
- Fixed uninitialized `SecondInstanceData.WorkingDirectory` on linux and windows ([#3154](https://github.com/wailsapp/wails/pull/3154)).
- Fixed save file dialog not appending file extension automatically on Windows by @almas1992 in [PR](https://github.com/wailsapp/wails/pull/3205)

## v2.7.1 - 2023-12-10

Expand Down