fix(editor): keep Cmd+Delete and Option+Delete in the editor instead of the data grid (#2022) - #2025
Merged
Merged
Conversation
…of the data grid (#2022)
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #2022.
The bug
In a query tab,
Cmd+Deletedid nothing after running aSELECT, and once you had clicked a result row it deleted that row instead of the editor line. Reporter's clue: it worked when a--comment preceded the query.Root cause
TablePro separates grid shortcuts from editor shortcuts with the editor's local
NSEventmonitor, which is gated on the editor being first responder and runs before the menu bar. That monitor only knows the editor's own combos (Cmd+/,Cmd+[,Cmd+],Cmd+Shift+D,Cmd+Shift+K, and friends).AppKit's own text bindings are not in that list.
Cmd+Delete(deleteToBeginningOfLine:) andOption+Delete(deleteWordBackward:) come fromStandardKeyBinding.dictand resolve at the very end of dispatch, inside the first responder'skeyDown→interpretKeyEvents. A main-menu key equivalent is matched long before that, so the Edit ▸ Delete item won every time. Its enabled state keyed on tab and selection state, never on focus:SELECT * FROM t;, nothing selected--comment firstTruncate TableonOption+Deletehad the identical shape. With tables selected in the sidebar, deleting a word in the editor marked them forTRUNCATEinstead.The fix
A focus gate on data-grid menu commands whose binding duplicates one of AppKit's standard text-editing bindings.
ShortcutAction.standardTextEditingBindingslists those bindings, read offStandardKeyBinding.dict, andshadowsStandardTextEditingBinding(_:)reports a collision for.dataGrid-context actions only. Editor and global actions keep their shortcut by design; the settings recorder now warns about them instead.MainContentCommandActionstracksfocusOwnsTextInputfrom anNSWindow.didUpdateNotificationobserver, coalesced to the next runloop turn and written only on a transition. The test isfirstResponder is NSTextInputClient, one predicate covering the SQL editor (CodeEditTextView.TextViewisNSView-based but conforms), a field editor over a grid cell, and the sidebar filter field.NSTableViewandNSOutlineVieware excluded, so selecting rows or sidebar tables changes nothing.View.dataGridShortcut(_:keyboard:yieldingTo:)applies the shortcut and drops it while the gate is on. It replacedoptionalKeyboardShortcutat the 13.dataGridmenu sites; it is inert at the 11 with no collision.A menu item consumes a matching key equivalent whether or not it is enabled, so disabling alone would have left
Cmd+Deletedoing nothing. Dropping the key equivalent is the only way to hand the keystroke back, and it matches what this file already does for theCmd+Froute (TableProApp.swift).Cmd+[/Cmd+]andCmd+Shift+Dwere checked and are not affected: the editor's local monitor already claims those before the menu sees them.Scope
No default binding changed.
Cmd+Deletefor delete-row matches Finder and Mail, and is correct once the gate is; a bareDeletecan never be a menu key equivalent because it would swallow backspace app-wide, and it already works through the grid's ownkeyDown.Tests
KeyboardShortcutTests: new reserved-shortcut cases and aStandard text-editing bindingssuite, including a guard that exactlydeleteandtruncateTableshadow a system binding by default.CommandActionsFocusGateTests: the gate yields forDeleteandTruncate Tableunder text focus, and never for a non-colliding grid action, an editor action, or an unbound action.QueryTabDeleteLineUITests: end-to-end through the real key-equivalent path, both halves of the reported repro. Note CI runs onlyTableProTests, so this one is local-only.swiftlint lint --strictis clean. I could not build in this worktree (noLibs/*.a), so please surface any compile errors.