Repository navigation
Make Cmd-P search tab titles by default - #3888
Closed
austinywang wants to merge 18 commits into
Closed
austinywang wants to merge 18 commits into
austinywang wants to merge 18 commits into
Conversation
Cmd-P already uses a shared switcher corpus and TabManager focus path for workspace and surface rows. The missing capability was policy: surface rows were only indexed after users discovered and enabled an opt-in setting. This changes the default so non-empty switcher queries include tab/panel surface rows while preserving the setting as an explicit opt-out. Constraint: Do not add a parallel go-to-tab mode when the existing switcher/focus owner already handles surface targets. Rejected: Add a separate tab palette | duplicates command palette search, ranking, and focus routing. Confidence: high Scope-risk: narrow Directive: Keep Cmd-P tab navigation on the shared switcher corpus and TabManager.focusTab(surfaceId:) path. Tested: git diff --check Not-tested: Local tests/build not run per instruction; CI will run tests.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR updates command palette search UI terminology by replacing "surfaces" with "tabs" across all localized strings and source code, and enables searching all surfaces by default by flipping a static boolean default from false to true. ChangesCommand Palette Terminology and Default Search Scope
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (14 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Merging latest origin/main avoids CI running the Cmd-P default change against stale IME and terminal focus updates already landed on the base branch. No conflict resolution was needed; this commit only records the upstream merge. Constraint: Iterate-PR workflow requires syncing the latest base before CI/feedback iteration. Confidence: high Scope-risk: narrow Directive: Treat this as a base-sync merge; feature behavior remains in the preceding commit. Tested: merge completed without conflicts Not-tested: Local tests/build not run per instruction; CI will run tests.
Contributor
Greptile SummaryFlips
Confidence Score: 5/5Safe to merge — the change is a single default-value flip backed by thorough localization updates and three well-isolated unit tests. The default flip from false to true is the only behavioral change; existing users who explicitly stored false are unaffected, and the new default is consistent across the settings UI, menu item, placeholder/empty-state copy, and search-alias index. No actor isolation, blocking-runtime, or SwiftUI state concerns are introduced. The new tests use isolated UserDefaults suites and verify the default path, explicit opt-out, and tab-title ranking end-to-end. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User opens Cmd-P Switcher] --> B{searchAllSurfacesEnabled?}
B -- "key absent (new default: true)" --> C[includesSurfaces = true]
B -- "key = true (explicit opt-in)" --> C
B -- "key = false (explicit opt-out)" --> D[includesSurfaces = false]
C --> E[Search workspaces + tab titles]
D --> F[Search workspaces only]
E --> G["Placeholder: 'Search workspaces and tabs'"]
F --> H["Placeholder: 'Search workspaces'"]
E --> I["Empty state: 'No workspaces or tabs match…'"]
F --> J["Empty state: 'No workspaces match…'"]
Reviews (13): Last reviewed commit: "fix: complete tab search localization" | Re-trigger Greptile |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 24037ad. Configure here.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Resources/Localizable.xcstrings`:
- Around line 42790-42874: The ellipsis usage is inconsistent: in the string
unit values for locales zh-Hans, zh-Hant, fr, ru, and th the stringUnit.value
uses three ASCII periods ("...") instead of the single Unicode ellipsis
(U+2026). Update those stringUnit.value entries (the translated values for
zh-Hans, zh-Hant, fr, ru, th) to replace the three periods with the single
ellipsis character “…”, and verify other locales’ stringUnit.value entries
remain unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 75e3d327-879d-41be-8a5f-bea78908615f
📒 Files selected for processing (6)
Resources/Localizable.xcstringsSources/ContentView.swiftSources/KeyboardShortcutSettings.swiftSources/SettingsNavigation.swiftSources/SettingsSearchAliases.swiftSources/cmuxApp.swift
This branch was successfully deployed
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
2 participants
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.
Summary
Testing
Closes #3724
Note
Medium Risk
Changes the default command-palette switcher search scope, which may affect user expectations and result ordering, but is limited to UI/search behavior with opt-out preserved.
Overview
Cmd-P’s switcher now includes tab titles in search by default by flipping
CommandPaletteSwitcherSearchSettings.defaultSearchAllSurfacestotrue, while keeping the existingcommandPalette.switcherSearchAllSurfacessetting as an opt-out.UI/UX text is updated throughout to reflect “tabs” terminology (placeholders/empty states, settings label/subtitle, and the File menu item now reading “Go to Workspace or Tab…”), including refreshed localization entries and settings-search aliases.
Adds unit tests covering the new default behavior, explicit opt-out, and that tab-title queries rank/select the expected tab entry.
Reviewed by Cursor Bugbot for commit 3631ab0. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Cmd‑P now searches tab titles by default in the switcher, so you can jump to tabs without a separate palette (fixes #3724).
Keeps
commandPalette.switcherSearchAllSurfacesas an opt‑out; renames the menu to “Go to Workspace or Tab…”, updates placeholders/empty states, and the setting label to “Command Palette Searches Tabs”; completes translations across supported locales and refreshes search aliases; adds tests for the default, explicit opt‑out, and tab‑title ranking; includes sidebar tools in results.Written for commit 3631ab0. Summary will update on new commits.
Summary by CodeRabbit
Changes
Tests