-
-
Notifications
You must be signed in to change notification settings - Fork 285
fix: add clipboard fallback for Write Mode selection reading (#259) #387
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.
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
Open
postoso
wants to merge
4
commits into
altic-dev:main
Choose a base branch
from
postoso:fix/259-writemode-clipboard
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
3fad60d
fix: add clipboard fallback for Write Mode selection reading (#259)
postoso b2be71e
fix: harden Write Mode clipboard fallback — late-copy restore + layou…
postoso f006a40
fix: coordinate clipboard-read fallback with TypingService pasteboard…
postoso c7fbc7f
Fix #259 clipboard fallback review findings
postoso File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| import Carbon.HIToolbox | ||
| import CoreGraphics | ||
| import Foundation | ||
|
|
||
| /// Resolves the virtual key code that produces a given character under the *current* | ||
| /// keyboard layout. | ||
| /// | ||
| /// Synthetic keyboard shortcuts (Cmd+C, Cmd+V) are posted by virtual key code, not by | ||
| /// character. A hard-coded ANSI key code (e.g. `kVK_ANSI_V` = 9) only lands on the right | ||
| /// physical key for QWERTY layouts; on Dvorak/AZERTY/QWERTZ the same key code produces a | ||
| /// different character, so the shortcut silently misfires. This looks up the key code for | ||
| /// the desired character in the active layout instead. | ||
| /// | ||
| /// Shared by `TypingService` (Cmd+V paste insertion) and `TextSelectionService` (Cmd+C | ||
| /// selection-read fallback) so both paths use one implementation rather than duplicating | ||
| /// the TIS / `UCKeyTranslate` scan. | ||
| enum LayoutAwareKeyCode { | ||
| /// Returns the virtual key code that produces `character` under the current keyboard | ||
| /// layout, falling back to `qwertyFallback` when the layout data is unavailable. | ||
| /// | ||
| /// Re-evaluated on every call so a runtime keyboard-layout switch is picked up | ||
| /// immediately. The underlying TIS API must run on the main thread, so the lookup is | ||
| /// dispatched there when called from a background thread. | ||
| static func virtualKeyCode(for character: Character, qwertyFallback: CGKeyCode) -> CGKeyCode { | ||
| if Thread.isMainThread { | ||
| return self.tisLookup(for: character, qwertyFallback: qwertyFallback) | ||
| } | ||
| var result = qwertyFallback | ||
| DispatchQueue.main.sync { | ||
| result = self.tisLookup(for: character, qwertyFallback: qwertyFallback) | ||
| } | ||
| return result | ||
| } | ||
|
|
||
| /// Performs the actual TIS + `UCKeyTranslate` scan. Must be called on the main thread. | ||
| private static func tisLookup(for character: Character, qwertyFallback: CGKeyCode) -> CGKeyCode { | ||
| guard let targetScalar = character.unicodeScalars.first else { return qwertyFallback } | ||
|
|
||
| guard let sourceRef = TISCopyCurrentKeyboardLayoutInputSource()?.takeRetainedValue(), | ||
| let rawPtr = TISGetInputSourceProperty(sourceRef, kTISPropertyUnicodeKeyLayoutData) | ||
| else { | ||
| return qwertyFallback | ||
| } | ||
| let layoutData = Unmanaged<CFData>.fromOpaque(rawPtr).takeUnretainedValue() as Data | ||
|
|
||
| return layoutData.withUnsafeBytes { buffer -> CGKeyCode in | ||
| guard let layoutPtr = buffer.baseAddress?.assumingMemoryBound(to: UCKeyboardLayout.self) else { | ||
| return qwertyFallback | ||
| } | ||
| var deadKeyState: UInt32 = 0 | ||
| var chars = [UniChar](repeating: 0, count: 4) | ||
| var length = 0 | ||
| let kbType = UInt32(LMGetKbdType()) | ||
|
|
||
| for keyCode: UInt16 in 0..<128 { | ||
| deadKeyState = 0 | ||
| length = 0 | ||
| let status = UCKeyTranslate( | ||
| layoutPtr, | ||
| keyCode, | ||
| UInt16(kUCKeyActionDisplay), | ||
| 0, | ||
| kbType, | ||
| UInt32(kUCKeyTranslateNoDeadKeysMask), | ||
| &deadKeyState, | ||
| chars.count, | ||
| &length, | ||
| &chars | ||
| ) | ||
| guard status == noErr, length > 0 else { continue } | ||
| if Unicode.Scalar(chars[0]) == targetScalar { | ||
| return CGKeyCode(keyCode) | ||
| } | ||
| } | ||
| return qwertyFallback | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| import Foundation | ||
|
|
||
| /// Serializes access to `NSPasteboard.general` across Fluid's synthetic copy/paste operations. | ||
| /// | ||
| /// Two subsystems mutate the shared pasteboard with snapshot/restore semantics: | ||
| /// - `TypingService` paste insertion temporarily writes the to-be-pasted text, posts Cmd+V, | ||
| /// then restores the user's clipboard on a background queue *after* a verification window | ||
| /// (up to several seconds later) — so its pasteboard "session" outlives the synchronous call. | ||
| /// - `TextSelectionService` selection-read fallback snapshots the clipboard, posts Cmd+C, | ||
| /// reads the copied selection, then restores the snapshot. | ||
| /// | ||
| /// Without coordination these overlap: a still-pending paste restore bumps `changeCount` | ||
| /// (and rewrites the pasteboard) while a selection read is polling for *its* Cmd+C, so the | ||
| /// previous insertion text gets mistaken for the freshly-copied selection and the wrong | ||
| /// snapshot is restored. This type is the single primitive both paths share to make their | ||
| /// pasteboard sessions mutually exclusive (issue #259). | ||
| /// | ||
| /// The session is a counting semaphore with value 1. A paste path holds it from the | ||
| /// synchronous setup through the async restore (handing the `signal()` off to `restoreQueue`); | ||
| /// the selection-read path holds it only for its own short critical section and always | ||
| /// releases before returning. | ||
| enum PasteboardSession { | ||
| /// One-at-a-time gate around `NSPasteboard.general` mutations. Mirrors the lock that | ||
| /// previously lived privately in `TypingService`; shared here so the selection-read path | ||
| /// composes with it instead of racing a parallel lock. | ||
| private static let semaphore = DispatchSemaphore(value: 1) | ||
|
|
||
| /// Serial queue on which `TypingService` performs its deferred pasteboard restore and | ||
| /// releases the session. Kept here so the session's lifetime (acquire → async restore → | ||
| /// release) is owned by one type. | ||
| static let restoreQueue = DispatchQueue(label: "PasteboardSession.Restore", qos: .utility) | ||
|
|
||
| /// Acquires the session, blocking until it is free. Used by the paste path, which runs on | ||
| /// a background queue and intentionally serializes behind any in-flight session. | ||
| static func beginExclusive() { | ||
| self.semaphore.wait() | ||
| } | ||
|
|
||
| /// Attempts to acquire the session within `timeoutMicros`. Returns `true` if acquired (the | ||
| /// caller then owns the session and must call `endExclusive()`), `false` on timeout (the | ||
| /// caller did **not** acquire it and must **not** call `endExclusive()`). | ||
| /// | ||
| /// Used by the main-thread selection-read fallback. The bounded timeout is load-bearing for | ||
| /// deadlock-freedom, not just UX: the paste path runs its paste closure *while holding the | ||
| /// session*, and that closure resolves the layout-aware key code, which does | ||
| /// `DispatchQueue.main.sync` when invoked off the main thread (see `LayoutAwareKeyCode`). So | ||
| /// during its brief key-code-lookup window a background paste holds the session *and* is | ||
| /// waiting on the main thread — if the main-thread selection read blocked on the session | ||
| /// forever, the two would wait on each other. The timeout breaks that cycle: on expiry the | ||
| /// selection read skips its clipboard fallback, the main run loop drains, the paste's | ||
| /// `main.sync` completes, and the session is released. Callers must not proceed to mutate | ||
| /// or sample the pasteboard unguarded after a timeout, because that re-opens the | ||
| /// cross-session race this guard exists to prevent. **Do not convert this to an unbounded | ||
| /// blocking wait: the timeout is what prevents the inversion from hanging.** | ||
| static func tryBeginExclusive(timeoutMicros: useconds_t) -> Bool { | ||
| let deadline = DispatchTime.now() + .microseconds(Int(timeoutMicros)) | ||
| return self.semaphore.wait(timeout: deadline) == .success | ||
| } | ||
|
|
||
| /// Releases the session previously acquired via `beginExclusive()` or a `true` result from | ||
| /// `tryBeginExclusive(timeoutMicros:)`. | ||
| static func endExclusive() { | ||
| self.semaphore.signal() | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import AppKit | ||
| import Foundation | ||
|
|
||
| /// A full-fidelity snapshot of an `NSPasteboard`'s contents (every item, every type). | ||
| /// | ||
| /// Used to save and restore the user's clipboard around synthetic copy/paste operations | ||
| /// so those operations don't clobber whatever the user had on the pasteboard. Shared by | ||
| /// `TypingService` (paste insertion) and `TextSelectionService` (Cmd+C selection fallback). | ||
| struct PasteboardSnapshot { | ||
| private struct ItemSnapshot { | ||
| let dataByType: [NSPasteboard.PasteboardType: Data] | ||
| } | ||
|
|
||
| private let items: [ItemSnapshot] | ||
|
|
||
| /// Captures the current contents of `pasteboard`. | ||
| static func capture(from pasteboard: NSPasteboard) -> PasteboardSnapshot { | ||
| let items: [ItemSnapshot] = pasteboard.pasteboardItems?.map { item in | ||
| var dataByType: [NSPasteboard.PasteboardType: Data] = [:] | ||
| for type in item.types { | ||
| if let data = item.data(forType: type) { | ||
| dataByType[type] = data | ||
| } | ||
| } | ||
| return ItemSnapshot(dataByType: dataByType) | ||
| } ?? [] | ||
| return PasteboardSnapshot(items: items) | ||
| } | ||
|
|
||
| /// Restores the captured contents onto `pasteboard`, replacing whatever is there. | ||
| func restore(to pasteboard: NSPasteboard) { | ||
| pasteboard.clearContents() | ||
| guard !self.items.isEmpty else { return } | ||
|
|
||
| let restoredItems = self.items.map { snap -> NSPasteboardItem in | ||
| let item = NSPasteboardItem() | ||
| for (type, data) in snap.dataByType { | ||
| item.setData(data, forType: type) | ||
| } | ||
| return item | ||
| } | ||
| _ = pasteboard.writeObjects(restoredItems) | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For input sources with a separate Command layer, such as macOS's Dvorak-QWERTY ⌘ layout, the key that produces
c/vwith no modifiers is not the key that produces the Copy/Paste menu equivalent while Command is down. Because this scan passes modifier state0but the callers later post the returned key with.maskCommand, the clipboard fallback can send a different shortcut instead of Cmd+C/Cmd+V on those layouts. Please translate using the Command modifier state that will be posted, or otherwise preserve the ANSI key for command-preserving layouts.Useful? React with 👍 / 👎.