Repository navigation
fix(app): wire or remove dead menu items and drop the New Session ⇧⌘S - #57919
Merged
Merged
Conversation
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.
Three macOS menu items pointed at command ids that nothing registers, so they did nothing. The New Session item also advertised ⇧⌘S.
session.new, ⇧⌘S (only registered inside a session)tab.new, ⌘N (registered globally;session.newalready forwards to it)project.open, ⌘O (not registered)sidebar.toggle, no shortcutsidebar.toggle, ⇧⌘Bproject.previous/project.next, ⌘⌥↑/↓ (not registered)session.newkeybindproject.select("Search projects", ⇧⌘O) is registered only on the new-session screen and is disabled when there are no projects, so the menu item would still do nothing on most screens.desktop.menu.openProject,previousProjectandnextProjectstay for now. Removing them touchesdesktop-native.ts, andlint:changedwould then flag 10 existing parser violations in that file.Test: each command the menu names is now a static definition (
{ id, keybind }) exported by the module that registers it and spread into itscommand.register(orctx.add(Command)for the terminal extension).desktop-menu.test.tsasserts every menucommandmatches one of those definitions, with extension commands formed as${extension}.${id}from the real extension definition. It does not cover conditional registration (sidebar.toggleonly with vertical tabs,logs.exportonly on desktop) or an owner dropping itsregistercall while keeping the export. Againstorigin/betait fails withsession.new,project.open,project.previousandproject.next.Verification:
bun typecheck(app, desktop),bun test src/shell/commands src/runtime/i18n/desktop-native.test.ts,bun run lint:changed origin/beta.