Skip to content

✨ Add unified JVM dialog parent - #624

Open
vinceglb wants to merge 2 commits into
mainfrom
codex/unified-jvm-dialog-parent
Open

✨ Add unified JVM dialog parent#624
vinceglb wants to merge 2 commits into
mainfrom
codex/unified-jvm-dialog-parent

Conversation

@vinceglb

Copy link
Copy Markdown
Owner

Summary

  • Add FileKitDialogParent as the canonical JVM dialog-parent API, with typed AWT, Windows HWND, X11, and Wayland portal-token factories.
  • Migrate FileKit-owned Compose, Windows, XDG, AWT/Swing fallback, and sample call sites to FileKitDialogSettings.parent.
  • Keep parentWindow as a deprecated one-release AWT source-compatibility adapter. It normalizes into parent; no constructor or copy overload accepts both values, so there is no hidden precedence rule.

Platform validation

  • Windows accepts AWT parents or non-zero HWNDs; the dedicated STA executor and FOS_FORCEFILESYSTEM behavior remain unchanged.
  • XDG portal parents use lowercase x11:<hex-XID> for X11 and accept opaque wayland:<token> values. Invalid/zero native handles and pointer-shaped Wayland strings are rejected.
  • Linux AWT/Swing fallback accepts only AWT parents; incompatible explicit native parents fail rather than becoming unparented.

Deliberate limitations

  • Wayland tokens must come from the caller's xdg-portal/xdg-foreign integration. FileKit does not accept or dereference raw wl_surface* pointers.
  • macOS JVM retains its existing NSOpenPanel/NSSavePanel.runModal() implementation. An AWT parent is accepted for API consistency, but FileKit does not bridge it to NSWindow or create an owned sheet.

Validation

  • ./gradlew :filekit-dialogs:check :filekit-dialogs-compose:check assemble --no-daemon
  • Focused JVM parent factory/resolver and compatibility tests
  • ktlint on changed non-Compose Kotlin files
  • git diff --check

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment on lines +30 to +33
public constructor(
title: String? = null,
parentWindow: Window?,
macOS: FileKitMacOSSettings = FileKitMacOSSettings(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

Comment on lines +13 to +15
internal class Awt(
val window: Window,
) : FileKitDialogParent()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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))"),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant