✨ Add unified JVM dialog parent - #624
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d7a800e49
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| public constructor( | ||
| title: String? = null, | ||
| parentWindow: Window?, | ||
| macOS: FileKitMacOSSettings = FileKitMacOSSettings(), |
There was a problem hiding this comment.
Keep null legacy construction unambiguous
Existing source such as FileKitDialogSettings("Choose", null) no longer compiles: null matches both the new primary constructor's FileKitDialogParent? parameter and this compatibility constructor's Window? parameter, producing an overload-resolution ambiguity. This was valid with the former nullable parentWindow constructor, so the one-release source-compatibility adapter needs an overload shape that preserves positional-null calls.
Useful? React with 👍 / 👎.
| internal class Awt( | ||
| val window: Window, | ||
| ) : FileKitDialogParent() |
There was a problem hiding this comment.
Preserve value equality for AWT parent wrappers
Because Awt uses identity equality, two settings independently created with the same window—such as FileKitDialogSettings(parentWindow = window)—now compare unequal, whereas the previous data class compared the stored Window references and considered them equal. This breaks the value semantics of FileKitDialogSettings for equality checks, sets, and caches; the wrapper should compare its contained window rather than its wrapper allocation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 094722a530
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| */ | ||
| @Deprecated( | ||
| message = "Use parent = FileKitDialogParent.awt(window) instead.", | ||
| replaceWith = ReplaceWith("FileKitDialogSettings(parent = FileKitDialogParent.awt(window))"), |
There was a problem hiding this comment.
Make the positional replacement nullable-safe
When window is declared as Window?, which this compatibility constructor explicitly accepts, applying the deprecation quick-fix rewrites the valid call to FileKitDialogParent.awt(window), but awt requires a non-null Window. The original constructor treats null as an unparented dialog, so the replacement should retain window?.let(FileKitDialogParent::awt) rather than leaving migrated code uncompilable.
Useful? React with 👍 / 👎.
Summary
FileKitDialogParentas the canonical JVM dialog-parent API, with typed AWT, Windows HWND, X11, and Wayland portal-token factories.FileKitDialogSettings.parent.parentWindowas a deprecated one-release AWT source-compatibility adapter. It normalizes intoparent; no constructor orcopyoverload accepts both values, so there is no hidden precedence rule.Platform validation
FOS_FORCEFILESYSTEMbehavior remain unchanged.x11:<hex-XID>for X11 and accept opaquewayland:<token>values. Invalid/zero native handles and pointer-shaped Wayland strings are rejected.Deliberate limitations
wl_surface*pointers.NSOpenPanel/NSSavePanel.runModal()implementation. An AWT parent is accepted for API consistency, but FileKit does not bridge it toNSWindowor create an owned sheet.Validation
./gradlew :filekit-dialogs:check :filekit-dialogs-compose:check assemble --no-daemonktlinton changed non-Compose Kotlin filesgit diff --check