feat(settings): make Find a rebindable shortcut so Cmd+F can open the filter bar - #2453
Conversation
|
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. |
|
Follow-up after re-reading the Codex job records properly (the earlier run's findings were in the job result, not stdout, and I had missed them). Codex adversarial-review returned
Reproduction: assign That context split was wrong on its own terms: This also closes a pre-existing hole the review surfaced: binding a data-grid action to Tests: 96 executed, 96 passed. Build and lint clean. Two review points I did not act on, with reasons:
|
A user asked to use
Cmd+Ffor the filter drawer instead of Find. This makes that possible without changing anyone's defaults.Root cause
Not a wrong default, a missing capability.
Find…was the only command wired outside the shortcut-customization system:EditMenuBuilder.swift:118-124hardcodedkeyEquivalent: "f", modifiers: .commandand passed noshortcut:/keyboard:, unlike its siblings Find Next and Find Previous.Cmd+Fsat inShortcutAction.reservedAppShortcuts, andreservedConflict(for:context:)returns that for every context, soShortcutConflictResolverrefused to bindCmd+Fto anything at all.So the request was blocked by a hard gate, not declined as a preference.
Why the defaults do not move
Apple's HIG lists
Command-F / Open a Find windowas a standard shortcut and says "In general, don't repurpose standard keyboard shortcuts for custom actions". Measured on macOS, Mail and Xcode both give filter a non-F key (Cmd+L,Cmd+Option+J).TablePlus binds
Cmd+Fto Open Row Filter and ships no grid find at all, which is where the request comes from. But TablePlus users are themselves asking for the opposite (TablePlus #2279, still open), Postico 2 shipsCmd+Option+Ffor the filter bar exactly as we do, and DataGrip, the only surveyed tool that has both a grid find and a filter, givesCmd+Fto Find.This shortcut has already flip-flopped three times (
CHANGELOG.md:2164→:1538→:658/:213), and the current arrangement is what the grid find bar with Search All Rows and the #2244 JSON-mode work were built for. A preference split between two camps is settled by rebinding, not by a fourth reversal.The change
ShortcutAction.find, context.global, defaultCmd+F..globalis explicit and commented, because only a global context overlaps.dataGrid, and that overlap is the entire reason the yield works.Cmd+Fremoved fromreservedAppShortcutsand fromeditorBuiltIns.Find…now routes throughKeyboardSettingslike every other customizable command.CodeEditSourceEditorlocal key monitor drops itscase (commandKey, "f").That last one is required, not cosmetic. The monitor runs before NSMenu key-equivalent matching and ignores
KeyboardSettings, so without it Settings would show Find on another key while the editor still opened Find onCmd+F. It is safe becauseTextViewController+FindActions.swiftalready declaresperformFindon the same controller, so the nil-targeted menu item reaches the identicalshowFindPanel()through the responder chain. Commitf5afbe221already addedCmd+Shift+DandCmd+Shift+Kto that same switch, so editing it is established practice here.One edit for the user.
shortcut(for:)already yields an action's default when another action's override claims that key in an overlapping context, so binding Toggle Filters toCmd+Fstands Find down on its own. Two menu items can never both holdCmd+F, which is the AppKit-blanks-the-loser trap inCLAUDE.md.Fixed while here
Review caught that the recorder's Reassign button called
clearShortcutunconditionally, persisting the cleared sentinel even when the losing action was still on its default.KeyboardShortcutModels.swift:390-392says explicitly that storing the stand-down is wrong: it makes it permanent, shows the action as customized, and its Reset arrow appears to work until the next launch re-applies the clear. In this feature's headline flow that meant a user who later moved the filter bar back toCmd+Option+Fwould find Find permanently unbound. Reassign now clears only a loser that has an explicit override; one on its default is left to yield and recovers on its own.Verification
verify.sh buildverify.sh test(14 menu + keyboard suites)verify.sh lint TablePro TableProTests LocalPackages/CodeEditSourceEditorverify.sh docsThe
categoryswitch is exhaustive with nodefault:, so the new case had to be handled at every site to compile at all.MainMenuShortcutCoverageTests.everyShortcutActionIsReachablepins that.findreaches exactly one menu item.Tests added: defaults still pin
.find==Cmd+Fand.toggleFilters==Cmd+Option+F(a guard against reversal four);Cmd+Fis no longer reserved in any context; Find yields and exactly one menu item ends up holdingCmd+F;Find…carries no hardcoded key equivalent; and Reassign leaves Find recoverable.No UI automation: the flow is Settings > Keyboard recorder input, and
ShortcutConflictResolverconsultsSystemHotkeyChecker, which reads live System Settings throughCopySymbolicHotKeys. Asserting on it would make the test depend on the machine, so the coverage sits on the deterministic app-owned layer instead.Notes for the reviewer
docs/features/keyboard-shortcuts.mdx:266pictures the Settings > Keyboard list, which now has a Find row. The screenshot needs re-capturing; I did not fabricate one.SyncCoordinator.swift:880). A user running this build and an older one with sync on, who rebindsCmd+F, gets a duplicate key equivalent on the old build, which still hardcodesCmd+Ffor Find.sanitized()only drops bare keys, so it does not filter this. Not fixable from this side and it heals when the other build updates.