ReviewOS

also looking at this

craft-native/craft

feat(android): the clipboard pair — ratchet 100 -> 98

#144
Merged glennmichael123 wants to merge feat/android-clipboard into main
6 files +414 -3
10 days ago

Ratchet 100 → 98. The first Android actions that need the Activity — getSystemService is a Context method — and the first with a null path that isn't an error.

getPrimaryClip() returning null is the normal case

Since Android 10, an app may only read the clipboard while it holds focus, or if it's the default IME. Everywhere else getPrimaryClip() answers null and the system logs a line the app can't suppress.

That's a routine platform restriction, not a failure. The Kotlin returns "" for it, and so does this. Reporting an error would turn a platform rule into a rejected promise on a page that did nothing wrong.

Three states collapse to the same empty string, and the Kotlin spells all three out:

  • no clip at all (the Android 10+ case)
  • a clip holding no items
  • an item whose getText() is null — reachable, since a ClipData can carry a URI or an Intent with no text representation

All four states are tested, with no JVM

A JNIEnv is a table of pointers, so the JVM can be written in Zig. read walks six Java objects and the fake covers every branch.

The trick that keeps it readable: GetMethodID returns the method name pointer as the jmethodID. The id is opaque to JNI and only ever handed back to a Call, so the fake reads it as a string and dispatches on it — one CallObjectMethod standing in for six real methods without a dispatch table of its own.

The third test is the one I'd keep if I could keep only one

It asserts the local frame is closed on every path. Three of the four return early, and a defer that ever moves below them leaks sixteen slots per call — and the JVM aborts the process on local-reference overflow rather than failing a call. That's a crash after enough reads, not a leak, and it wouldn't reproduce until a user had copied a few things.

mutationresult
double-pop on the null-clip paththe frame test fails
null clip treated as an errorthe empty-string test fails

Two things kept deliberately

  • CLIPBOARD_SERVICE is read off Context, not spelled as "clipboard". The value has been stable since API 1 so hardcoding would work — but a constant copied into another language is one that can only ever be wrong later, and reading it costs one static field access on a path already making five calls.
  • The ClipData label stays "Craft". Android 13+ shows it to the user in a preview toast, so changing it because Zig now does the writing would change what they see.

Null vs empty across the seam

The Kotlin now depends on these being different answers: null is "Zig declined", "" is "the clipboard is empty". Collapsing them would make an empty clipboard fall through and be read twice.

zig build test:android → 67 tests, 11/11 steps · both .so build · generator 10/10 · pickier clean.

6 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.