[v3] Fix save dialog filename not pre-populated on Linux (#4841)#4842
Conversation
The SaveFileDialog.SetFilename() option had no effect on Linux because the filename was never passed to the GTK file chooser. This adds a currentName parameter to runChooserDialog and calls gtk_file_chooser_set_current_name() for save dialogs when a filename is specified. Changes: - linux_cgo.go: Add currentName parameter and GTK call - linux_purego.go: Add gtkFileChooserSetCurrentName binding and same fix Fixes the Linux implementation to match the behaviour on Windows and macOS.
|
Caution Review failedThe pull request is closed. WalkthroughAdds passing of a save-filename to the Linux file chooser: runChooserDialog now accepts a currentName and calls gtk_file_chooser_set_current_name for Save dialogs; runSaveFileDialog forwards dialog.filename. Also adds changelog entries documenting related drag-and-drop fixes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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. Comment |
|
@CodeRabbit base branch changed. Please review. |
|
@leaanthony: I'll review the changes now that the base branch has been updated. ✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
v3/UNRELEASED_CHANGELOG.md (1)
26-26: Changelog entry looks good.The entry is well-formatted, includes the issue reference and author attribution, and follows the "Keep a Changelog" guidelines. Consider slightly more descriptive phrasing for clarity (e.g., "Fix SaveFileDialog.SetFilename() not pre-populating the filename on Linux"), but the current entry is entirely acceptable.
v3/pkg/application/linux_purego.go (1)
1061-1065: Core fix implemented correctly for purego.The implementation properly pre-populates the save dialog filename using the registered GTK function pointer.
Optional: Move constant to global scope
The
GtkFileChooserActionSaveconstant is defined locally at line 1062. For consistency with other GTK constants and potential reusability, consider moving it to the global constants section (around lines 28-63):const ( nilPointer pointer = 0 ) const ( GSourceRemove int = 0 + // https://gitlab.gnome.org/GNOME/gtk/-/blob/gtk-3-24/gtk/gtkfilechooser.h + GtkFileChooserActionOpen = 0 + GtkFileChooserActionSave = 1 + GtkFileChooserActionSelectFolder = 2 + // https://gitlab.gnome.org/GNOME/gtk/-/blob/gtk-3-24/gdk/gdkwindow.h#L121 GdkHintMinSize = 1 << 1Then remove the local definition at line 1062.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
v3/UNRELEASED_CHANGELOG.mdv3/pkg/application/linux_cgo.gov3/pkg/application/linux_purego.go
🧰 Additional context used
🧬 Code graph analysis (2)
v3/pkg/application/linux_cgo.go (1)
v3/pkg/application/dialogs.go (1)
FileFilter(164-167)
v3/pkg/application/linux_purego.go (1)
v3/pkg/application/dialogs.go (1)
FileFilter(164-167)
🔇 Additional comments (9)
v3/pkg/application/linux_cgo.go (4)
1725-1725: LGTM: Function signature extended correctly.The addition of the
currentNameparameter torunChooserDialogis well-placed and clearly named.
1775-1782: LGTM: Core fix implemented correctly.The implementation properly:
- Pre-populates the filename only for save dialogs
- Uses the correct GTK API
gtk_file_chooser_set_current_name- Manages memory correctly (allocates with
C.CStringand frees withC.free)- Includes clear documentation
1857-1858: LGTM: Open dialog correctly passes empty filename.Open file dialogs correctly pass an empty string for
currentNamesince they don't need filename pre-population.
1940-1941: LGTM: Save dialog correctly passes filename for pre-population.This change enables the core functionality described in the PR: pre-populating the filename field in save dialogs by passing
dialog.filenametorunChooserDialog.v3/pkg/application/linux_purego.go (5)
155-155: LGTM: Function pointer declared correctly.The
gtkFileChooserSetCurrentNamefunction pointer is properly declared with the correct signature matching the GTK C API.
309-309: LGTM: Function pointer registered correctly.The registration properly binds the Go function pointer to the GTK library symbol
gtk_file_chooser_set_current_name.
1029-1029: LGTM: Function signature extended correctly.The addition of the
currentNameparameter matches the CGO implementation and maintains consistency between both Linux variants.
1136-1137: LGTM: Open dialog correctly passes empty filename.Consistent with the CGO implementation, open file dialogs pass an empty string since they don't need filename pre-population.
1218-1219: LGTM: Save dialog correctly passes filename for pre-population.This change enables the core functionality for the purego implementation, matching the CGO version and fulfilling the PR objective.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
v3/pkg/application/linux_purego.go (1)
1130-1141: Fix hardcoded action constant in runOpenFileDialog.Line 1137 passes the hardcoded constant
GtkFileChooserActionOpeninstead of the computedactionvariable. This breaks directory selection functionality—whendialog.canChooseDirectoriesis true, the function should passGtkFileChooserActionSelectFolder(value 2), but it always passesGtkFileChooserActionOpen(value 0).The CGO implementation (linux_cgo.go, line 2130) correctly passes the computed
actionvariable. Change line 1137 fromGtkFileChooserActionOpen,toaction,to match the intended behavior.
🧹 Nitpick comments (1)
v3/pkg/application/linux_purego.go (1)
1032-1068: LGTM! Implementation correctly pre-populates the filename.The logic correctly calls
gtk_file_chooser_set_current_name()only for Save dialogs when a filename is provided, which aligns with GTK documentation and fixes the reported issue.Minor: Consider extracting the constant to package level.
The constant
GtkFileChooserActionSaveis defined at both Line 1065 and Line 1205. Consider defining it once at the package level alongside other GTK constants (around lines 32-63) to avoid duplication.🔎 Proposed refactor to eliminate constant duplication
Define the constant at package level (around line 63):
GtkMessageWarning = 1 GtkMessageQuestion = 2 GtkMessageError = 3 + + // https://gitlab.gnome.org/GNOME/gtk/-/blob/gtk-3-24/gtk/gtkfilechooser.h + GtkFileChooserActionOpen = 0 + GtkFileChooserActionSave = 1 + GtkFileChooserActionSelectFolder = 2 )Then remove the local definitions at lines 1065, 1106-1107, and 1205.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
v3/UNRELEASED_CHANGELOG.mdv3/pkg/application/linux_cgo.gov3/pkg/application/linux_purego.go
✅ Files skipped from review due to trivial changes (1)
- v3/UNRELEASED_CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
- v3/pkg/application/linux_cgo.go
🧰 Additional context used
🧬 Code graph analysis (1)
v3/pkg/application/linux_purego.go (1)
v3/pkg/application/dialogs.go (1)
FileFilter(164-167)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
- GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
- GitHub Check: Run Go Tests v3 (macos-latest, 1.24)
🔇 Additional comments (4)
v3/pkg/application/linux_purego.go (4)
155-155: LGTM! Function pointer declaration is correct.The
gtkFileChooserSetCurrentNamefunction pointer follows the established pattern for GTK function declarations in this file.
309-309: LGTM! Function registration is correct.The purego registration for
gtk_file_chooser_set_current_nameis properly placed and follows the correct pattern.
1139-1140: LGTM! Correctly passes empty string for open dialogs.Open dialogs don't need a pre-populated filename, so passing an empty string is correct.
1221-1222: LGTM! This is the key fix for the reported issue.Passing
dialog.filenameas thecurrentNameparameter correctly propagates the filename set viaSetFilename()to the GTK file chooser, fixing issue #4841.
|
…) (wailsapp#4842) The SaveFileDialog.SetFilename() option had no effect on Linux because the filename was never passed to the GTK file chooser. This adds a currentName parameter to runChooserDialog and calls gtk_file_chooser_set_current_name() for save dialogs when a filename is specified. Changes: - linux_cgo.go: Add currentName parameter and GTK call - linux_purego.go: Add gtkFileChooserSetCurrentName binding and same fix Fixes the Linux implementation to match the behaviour on Windows and macOS. Co-authored-by: sas229 <sas229@cam.ac.uk> Co-authored-by: Lea Anthony <lea.anthony@gmail.com>
Description
The
SaveFileDialog.SetFilename()option has no effect on Linux because the filename is never passed to the GTK file chooser. This fix adds support for pre-populating the filename field by callinggtk_file_chooser_set_current_name().Fixes #4841
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
Wails (v3.0.0-dev) Wails Doctor
System
┌────────────────────────────────────────────────────────────────────────────────────────┐
| Name | Arch Linux |
| Version | Unknown |
| ID | arch |
| Branding | |
| Platform | linux |
| Architecture | amd64 |
| CPU | Intel(R) Core(TM) i9-8950HK CPU @ 2.90GHz |
| GPU 1 | GP107GLM [Quadro P2000 Mobile] (NVIDIA Corporation) - Driver: nouveau |
| GPU 2 | CoffeeLake-H GT2 [UHD Graphics 630] (Intel Corporation) - Driver: i915 |
| Memory | 31GB |
└────────────────────────────────────────────────────────────────────────────────────────┘
Build Environment
┌─────────────────────────────────────────────────────────────────────────────────────────────────────┐
| Wails CLI | v3.0.0-dev |
| Go Version | go1.25.3 X:nodwarf5 |
| Revision | be60783 |
| Modified | false |
| -buildmode | exe |
| -compiler | gc |
| CGO_CFLAGS | |
| CGO_CPPFLAGS | |
| CGO_CXXFLAGS | |
| CGO_ENABLED | 1 |
| CGO_LDFLAGS | |
| DefaultGODEBUG | containermaxprocs=0,decoratemappings=0,tlssha1=1,updatemaxprocs=0,x509sha256skid=0 |
| GOAMD64 | v1 |
| GOARCH | amd64 |
| GOEXPERIMENT | nodwarf5 |
| GOOS | linux |
| vcs | git |
| vcs.modified | false |
| vcs.revision | be60783 |
| vcs.time | 2025-09-08T08:42:14Z |
└─────────────────────────────────────────────────────────────────────────────────────────────────────┘
Dependencies
┌──────────────────────────────────────────┐
| gcc | 15.2.1+r301+gf24307422d1d-1 |
| gtk3 | 1:3.24.51-1 |
| npm | 11.6.4 |
| pkg-config | 2.5.1-1 |
| webkit2gtk | 2.50.4-1 |
| |
└──────── * - Optional Dependency ─────────┘
Checking for issues
SUCCESS No issues found
Diagnosis
SUCCESS Your system is ready for Wails development!
Need documentation? Run: wails3 docs
♥ If Wails is useful to you or your company, please consider sponsoring the project: wails3 sponsor
Checklist:
UNRELEASED_CHANGELOG.mdwith details of this PRSummary by CodeRabbit
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.