feat: settings manager with two-scope UI and keybinding system#11
Merged
Conversation
Add a VS Code-style settings panel (Cmd+,) with User and Workspace scopes. Schema-driven architecture makes adding new settings a one-line change. Includes IPC handlers for per-layer read/write, theme picker, search, modified indicators, and placeholder categories for future features. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Commands are now pure {id, label, category} with no keybinding knowledge.
Keybindings are a separate ordered rule list evaluated with last-match-wins
priority, enabling context-dependent dispatch via `when` clauses, multiple
bindings per command, and negative rules to suppress defaults.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR adds a VS Code-style KeybindingService, migrates keybinding storage to ordered rule overrides, and implements a two-scope (user/workspace) settings system with IPC persistence, preload APIs, renderer hooks, and a searchable Settings pane with keybinding editing and recording. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Settings UI
participant Hook as useSettings Hook
participant IPC as Renderer IPC
participant Main as Main Process
participant Store as Electron Store / .aide
UI->>Hook: setValue(key, value) / setScope(scope)
Hook->>Hook: optimistic update (user/workspace layer)
Hook->>IPC: window.api.setUserSetting / setWorkspaceSetting
IPC->>Main: IPC_SETTINGS_SET_USER / IPC_SETTINGS_SET_WORKSPACE
Main->>Store: write user (electron-store) or workspace (.aide/settings.json)
Main->>Main: recompute resolved settings
Main->>IPC: broadcast SETTINGS_CHANGED(resolved)
IPC->>Hook: onSettingsChanged callback
Hook->>UI: update resolved and re-render
sequenceDiagram
participant Recorder as KeybindingRecorder
participant UI as Settings UI
participant Hook as useKeybindingOverrides
participant Service as KeybindingService
participant IPC as Renderer IPC
participant Main as Main Process
Recorder->>Service: getAllKeybindingRules()
Service-->>Recorder: rules + suppressed metadata
Recorder->>Recorder: detect conflicts, show hints
UI->>Recorder: onRecord(newKey)
Recorder->>Hook: addOverride({key, command, when})
Hook->>IPC: window.api.setKeybindingOverrides(updated)
IPC->>Main: IPC_KEYBINDINGS_SET
Main->>Main: persist to electron-store
Main->>IPC: broadcast KEYBINDINGS_CHANGED
IPC->>Service: loadKeybindings(defaults, updated)
Service->>Service: parse, suppress negatives, cache, apply
Recorder->>UI: updated binding shown
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (4)
packages/main/src/settingsResolver.ts (1)
90-103: Minor redundancy in merge logic.
userDefaultsis fetched again at line 90, butappDefaults(fromresolveAppDefaults) already incorporates user defaults. The fallback chainprojectSettings.X ?? userDefaults.X ?? appDefaults.Xis equivalent toprojectSettings.X ?? appDefaults.XsinceappDefaultsalready containsuserDefaults ?? BUILT_IN_DEFAULTS.This doesn't cause incorrect behavior (the same value is selected either way), but the intermediate
userDefaultslookup is redundant.♻️ Simplified merge (optional)
export async function resolveSettings( rootPath: string, store: Store<AppSettings>, ): Promise<ResolvedSettings> { const projectSettings = await readProjectSettings(rootPath) const appDefaults = resolveAppDefaults(store) - const userDefaults = (store.get('editorDefaults') ?? {}) as Partial<AideProjectSettings> // Merge: project > user > built-in return { - tabSize: projectSettings.tabSize ?? userDefaults.tabSize ?? appDefaults.tabSize, + tabSize: projectSettings.tabSize ?? appDefaults.tabSize, - insertSpaces: - projectSettings.insertSpaces ?? userDefaults.insertSpaces ?? appDefaults.insertSpaces, + insertSpaces: projectSettings.insertSpaces ?? appDefaults.insertSpaces, // ... apply same pattern to other scalar fields🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/main/src/settingsResolver.ts` around lines 90 - 103, The merge is redundant because appDefaults (from resolveAppDefaults) already includes userDefaults, so remove the intermediate userDefaults fallback: stop reading store.get('editorDefaults') into userDefaults and change the merge expressions in settingsResolver.ts that use projectSettings.* ?? userDefaults.* ?? appDefaults.* to simply projectSettings.* ?? appDefaults.* (update all occurrences like tabSize, insertSpaces, wordWrap, rulers, fontSize, fontFamily, formatOnSave) and remove the unused userDefaults variable.packages/renderer/src/components/CommandPalette.tsx (1)
33-42: Consider adding dependencies touseMemofor dynamic keybinding updates.The
useMemohas an empty dependency array, so items are computed once on mount. If keybindings can change at runtime (e.g., user modifies a keybinding in settings while the palette is open), the displayed shortcuts won't update.If keybinding changes are expected to require reopening the palette, this is fine. Otherwise, consider exposing a signal or version counter from
KeybindingServiceto invalidate the memo.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/components/CommandPalette.tsx` around lines 33 - 42, The memoized mapping that builds palette items (inside useMemo) ignores changes to keybindings, so update the useMemo dependency array to include a reactive keybinding-change signal/version from your KeybindingService (or a prop/state that updates when bindings change) so getKeybindingsForCommand and formatKeybinding are re-run when keybindings change; locate the useMemo that maps sorted -> { id, label, description } and add the KeybindingService signal/version (or a keybindingsVersion state) to its dependencies, or subscribe to keybinding updates and force an update that invalidates the memo.packages/renderer/src/components/settings/SettingControl.tsx (1)
31-34: Clearing number input sets value to 0 instead of allowing empty state.
Number('')evaluates to0, notNaN. When a user clears the input field, the value becomes0instead of remaining empty or becomingundefined. This might interfere with the "reset to default" workflow if defaults differ from0.Consider allowing empty values to represent "unset":
♻️ Optional: Allow empty values
onChange={(e) => { + if (e.target.value === '') { + onChange(undefined) + return + } const num = Number(e.target.value) if (!Number.isNaN(num)) onChange(num) }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/components/settings/SettingControl.tsx` around lines 31 - 34, The current onChange handler uses Number(e.target.value) which turns an empty string into 0; change the handler in SettingControl's onChange so it explicitly checks for an empty string first and treats that as "unset" (e.g., call onChange(undefined) or onChange(null) per your app conventions), otherwise parse the value to a number and call onChange(num) only when Number.isNaN(num) is false; update any related prop typings if necessary to accept the unset value.packages/renderer/src/styles/settings-pane.css (1)
556-560: Use CSS variable for conflict color to support theming.The conflict message uses a hardcoded color
#e5534binstead of a CSS variable token. This won't adapt to theme changes.♻️ Proposed fix
.keybinding-recorder__conflict { font-size: 11px; - color: `#e5534b`; + color: var(--text-error); padding: 0 2px; }As per coding guidelines: "Use CSS variable token system with
data-themeattribute for theming, supporting dark and light mode"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/styles/settings-pane.css` around lines 556 - 560, Replace the hardcoded color in the .keybinding-recorder__conflict rule with the theming CSS variable token (e.g. color: var(--color-error, `#e5534b`)); update the rule so it uses var(...) instead of `#e5534b` and ensure the token is provided by the existing data-theme variable system (so dark/light themes override it); modify the .keybinding-recorder__conflict selector only to reference the CSS variable while keeping font-size and padding intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/renderer/src/components/AppShell.tsx`:
- Around line 489-492: The IPC call window.api.getKeybindingOverrides() lacks
error handling so failures are swallowed; update the initialization to handle
rejection by adding a .catch() that logs the error (e.g., console.error or the
app's logger) and then calls loadKeybindings(defaultKeybindings, {}) or
loadKeybindings(defaultKeybindings, undefined) to ensure defaults still load.
Keep the existing success path (loadKeybindings(defaultKeybindings, overrides))
and reference window.api.getKeybindingOverrides, loadKeybindings, and
defaultKeybindings when making the change.
In `@packages/renderer/src/components/panes/SettingsPane.tsx`:
- Around line 21-24: The check in the useEffect that sets workspace availability
only treats null as "no workspace" but getWorkspaceRoot() can return undefined;
update the predicate used in the window.api.getWorkspaceRoot().then callback
(where setWorkspaceAvailable is called) to treat both null and undefined as "no
workspace" (e.g., use a null/undefined-safe check such as root != null or
Boolean(root)) so the Workspace scope won't show as available when
getWorkspaceRoot() is unset.
In `@packages/renderer/src/components/settings/KeyboardShortcutsTable.tsx`:
- Around line 135-143: The current recording state uses recordingCommandId and
compares it to row.commandId, causing the KeybindingRecorder to appear for all
rows sharing the same commandId; change the recording state to a composite
identifier (e.g., recordingKey = `${row.commandId}-${row.key}-${row.when}` or
use the row index) and update all uses: replace recordingCommandId with the
composite (both where you render KeybindingRecorder and where you call
setRecordingCommandId) so each row has its own independent recording ID; ensure
onRecord calls addOverride({ key: kb, command: row.commandId, when: row.when })
as needed and reset the composite state to null with setRecordingCommandId(null)
(or setRecordingKey(null)) after recording or canceling.
In `@packages/renderer/src/components/settings/SettingsContent.tsx`:
- Around line 24-27: The code switches to search mode when searchQuery is truthy
even if it’s only whitespace; update the logic in SettingsContent to trim the
query first (e.g., const trimmed = searchQuery?.trim()) and use that trimmed
value to decide whether to call getSearchResults(trimmed) or fall back to
getCategorySections(activeCategory), so whitespace-only queries are treated as
empty and show categories instead of "No settings found."
- Around line 37-61: Special sections (the Color Theme row and
KeyboardShortcutsTable) are rendering even when a search is active and the
special UI blocks themselves don't match the query; update the render guards so
these blocks are only shown when search is not active or when the special item
explicitly matches the query. Concretely, wrap the existing checks for
section.categoryId === 'workbench.appearance' (the Theme select using theme and
onThemeChange) and section.categoryId === 'keyboardShortcuts'
(KeyboardShortcutsTable) with an additional condition such as !isSearchActive ||
matchesSpecialItem(section, searchQuery) (or wire in the existing
searchQuery/isSearchActive state) so the theme row and KeyboardShortcutsTable
only render when appropriate. Ensure to implement a small helper
matchesSpecialItem that tests the query against the special block
labels/descriptions if you choose explicit matching.
In `@packages/renderer/src/hooks/useKeybindingOverrides.ts`:
- Around line 36-44: addOverride and removeOverride build their new lists from
the render-time overrides array which can race and clobber concurrent updates;
change them to compute the new list from the latest persisted state instead of
the captured `overrides`. Specifically, in `addOverride` and `removeOverride`
(currently referencing `overrides` and calling `persist(updated)`), first fetch
the current persisted overrides (or use `persist`'s updater/callback form if
available) and then derive the new list (e.g., current.concat(rule) for add, or
filter the current list for remove) before calling `persist` so updates are
applied against the latest state rather than the stale render snapshot. Ensure
you still keep `useCallback` and its dependencies accurate (remove `overrides`
if you no longer reference it inside the callback).
- Around line 18-23: The initial keybinding load in the useEffect using
window.api.getKeybindingOverrides() can reject and leave loading true; wrap the
async call in a try/finally so setLoading(false) always runs, and in the catch
fallback to defaults by calling loadKeybindings(defaultKeybindings, {}) (or
similar) and setOverrides to an empty/default value; update the useEffect (and
its inner async function) around getKeybindingOverrides, setOverrides,
loadKeybindings, and setLoading to ensure failures are caught and defaults are
applied.
In `@packages/renderer/src/hooks/useSettings.ts`:
- Around line 31-58: The initial load() and the onSettingsChanged callback don't
handle rejected IPC promises, which can leave loading stuck true and produce
unhandled rejections; wrap the Promise.all call inside load() in a
try/catch/finally (check cancelled after await as now) so you
setBuiltInDefaults/setUserSettings/setWorkspaceSettings/setResolved only on
success, log the error in catch, and always setLoading(false) in finally; for
the subscription handler (window.api.onSettingsChanged) ensure the downstream
fetches (window.api.getUserSettings and window.api.getWorkspaceSettings) are
handled with .catch(...) or async/try-catch so failures are caught and logged
instead of causing unhandled rejections while still updating resolved via
setResolved(newResolved).
- Around line 62-91: The optimistic updates in setValue and resetToDefault
mutate local state via setUserSettings/setWorkspaceSettings before awaiting
window.api.setUserSetting/setWorkspaceSetting, but there is no reconciliation on
failure; wrap the API calls in try/catch and on error revert the optimistic
state to the previous value (capture prev inside the updater or store a snapshot
before updating), restore that snapshot in the catch block, and propagate or
surface the error; update both setValue and resetToDefault to follow this
pattern so local state stays consistent with persisted state.
In `@packages/renderer/src/lib/KeybindingService.ts`:
- Around line 219-228: The chord prefix branch is swallowing the first key even
when all matching chord rules are disabled; before calling
e.preventDefault()/e.stopPropagation() and setting pendingChord, evaluate the
rule's activation condition (the `when`/context for the rule) so you only enter
chord mode for rules that are actually enabled. Concretely, in the loop over
`rules` (inspect `entry`, `entry.parts`, and `partsMatch`) add a check that the
rule's `when`/context is satisfied (use the existing context-evaluation helper
or method used elsewhere in KeybindingService) and skip entries whose `when` is
false before assigning `pendingChord` and touching the event. Ensure you still
respect `entry.suppressed` and `entry.parts.length === 2` logic.
- Around line 135-157: loadKeybindings currently overwrites the shared rules
array which drops component-scoped/live rules added by useKeybinding; change
loadKeybindings to merge resolved rules with existing rules instead of replacing
them: filter the existing global rules (the variable rules) to keep any entries
with the component-scoped/source marker used by useKeybinding (e.g., source ===
'component' or a live flag), merge them with the newly computed resolved array
while deduplicating by rule.key (or rule id) so defaults/user overrides still
supersede, then run processRemovals and assign the merged array back to rules;
update logic around parse(rule.key) and processRemovals to operate on the merged
set to preserve lifecycle-scoped shortcuts.
- Around line 55-65: parseSingle currently collapses Cmd and Ctrl into a single
meta flag and leaves key strings unnormalized; update parseSingle (and the
corresponding multi-segment parsing at the other block) to return separate
booleans for cmd (metaKey) and ctrl (ctrlKey) instead of a single meta, and
normalize the parsed key token to the browser KeyboardEvent.key canonical values
(e.g., use " " for Space, "ArrowUp"/"ArrowDown"/"ArrowLeft"/"ArrowRight" for
arrows, map aliases like "enter" -> "Enter", "esc" -> "Escape", and ensure
correct casing) so later comparisons against event.key succeed; keep shift and
alt detection but include opt alias for alt, and ensure the returned
ShortcutParts shape is updated to include ctrl and cmd fields used by the rest
of the code.
In `@packages/renderer/src/lib/settingsSchema.ts`:
- Around line 126-127: getCategoriesWithSettings currently builds categories
only from SETTINGS_DESCRIPTORS, omitting special non-descriptor pages; update
the function to also include category values from your non-descriptor pages
collection (e.g., SETTINGS_PAGES, SETTINGS_NON_DESCRIPTOR_PAGES, or whatever
array/object holds the special pages) by iterating that collection and adding
each page.category into the same Set before returning it so the sidebar treats
those sections as non-empty; keep getCategoriesWithSettings and
SETTINGS_DESCRIPTORS as the primary anchors when making the change.
---
Nitpick comments:
In `@packages/main/src/settingsResolver.ts`:
- Around line 90-103: The merge is redundant because appDefaults (from
resolveAppDefaults) already includes userDefaults, so remove the intermediate
userDefaults fallback: stop reading store.get('editorDefaults') into
userDefaults and change the merge expressions in settingsResolver.ts that use
projectSettings.* ?? userDefaults.* ?? appDefaults.* to simply projectSettings.*
?? appDefaults.* (update all occurrences like tabSize, insertSpaces, wordWrap,
rulers, fontSize, fontFamily, formatOnSave) and remove the unused userDefaults
variable.
In `@packages/renderer/src/components/CommandPalette.tsx`:
- Around line 33-42: The memoized mapping that builds palette items (inside
useMemo) ignores changes to keybindings, so update the useMemo dependency array
to include a reactive keybinding-change signal/version from your
KeybindingService (or a prop/state that updates when bindings change) so
getKeybindingsForCommand and formatKeybinding are re-run when keybindings
change; locate the useMemo that maps sorted -> { id, label, description } and
add the KeybindingService signal/version (or a keybindingsVersion state) to its
dependencies, or subscribe to keybinding updates and force an update that
invalidates the memo.
In `@packages/renderer/src/components/settings/SettingControl.tsx`:
- Around line 31-34: The current onChange handler uses Number(e.target.value)
which turns an empty string into 0; change the handler in SettingControl's
onChange so it explicitly checks for an empty string first and treats that as
"unset" (e.g., call onChange(undefined) or onChange(null) per your app
conventions), otherwise parse the value to a number and call onChange(num) only
when Number.isNaN(num) is false; update any related prop typings if necessary to
accept the unset value.
In `@packages/renderer/src/styles/settings-pane.css`:
- Around line 556-560: Replace the hardcoded color in the
.keybinding-recorder__conflict rule with the theming CSS variable token (e.g.
color: var(--color-error, `#e5534b`)); update the rule so it uses var(...) instead
of `#e5534b` and ensure the token is provided by the existing data-theme variable
system (so dark/light themes override it); modify the
.keybinding-recorder__conflict selector only to reference the CSS variable while
keeping font-size and padding intact.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c8e5a219-62d1-484d-8cf6-ca0bbf9543d5
📒 Files selected for processing (28)
IDE_BUILD_PLAN.mdpackages/main/src/index.tspackages/main/src/preload.tspackages/main/src/settingsResolver.tspackages/renderer/src/components/AppShell.tsxpackages/renderer/src/components/CommandPalette.tsxpackages/renderer/src/components/DockviewContainer.tsxpackages/renderer/src/components/panes/SettingsPane.tsxpackages/renderer/src/components/settings/KeybindingRecorder.tsxpackages/renderer/src/components/settings/KeyboardShortcutsTable.tsxpackages/renderer/src/components/settings/SettingControl.tsxpackages/renderer/src/components/settings/SettingRow.tsxpackages/renderer/src/components/settings/SettingsCategorySidebar.tsxpackages/renderer/src/components/settings/SettingsContent.tsxpackages/renderer/src/components/settings/SettingsHeader.tsxpackages/renderer/src/hooks/useKeybindingOverrides.tspackages/renderer/src/hooks/useSettings.tspackages/renderer/src/lib/CommandRegistry.tspackages/renderer/src/lib/KeybindingService.tspackages/renderer/src/lib/ShortcutManager.tspackages/renderer/src/lib/defaultCommands.tspackages/renderer/src/lib/defaultKeybindings.tspackages/renderer/src/lib/formatKeybinding.tspackages/renderer/src/lib/settingsSchema.tspackages/renderer/src/main.tsxpackages/renderer/src/styles/settings-pane.csspackages/shared/src/commands.tspackages/shared/src/index.ts
💤 Files with no reviewable changes (1)
- packages/renderer/src/lib/ShortcutManager.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{css,scss,tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Use CSS variable token system with
data-themeattribute for theming, supporting dark and light mode
Files:
packages/renderer/src/main.tsxpackages/renderer/src/components/DockviewContainer.tsxpackages/main/src/settingsResolver.tspackages/shared/src/commands.tspackages/renderer/src/lib/formatKeybinding.tspackages/renderer/src/components/CommandPalette.tsxpackages/renderer/src/styles/settings-pane.csspackages/renderer/src/lib/defaultCommands.tspackages/renderer/src/lib/defaultKeybindings.tspackages/renderer/src/components/settings/KeyboardShortcutsTable.tsxpackages/renderer/src/components/AppShell.tsxpackages/renderer/src/components/settings/SettingsHeader.tsxpackages/renderer/src/components/panes/SettingsPane.tsxpackages/renderer/src/components/settings/SettingRow.tsxpackages/main/src/index.tspackages/renderer/src/components/settings/SettingControl.tsxpackages/renderer/src/lib/settingsSchema.tspackages/renderer/src/lib/KeybindingService.tspackages/renderer/src/components/settings/KeybindingRecorder.tsxpackages/renderer/src/components/settings/SettingsContent.tsxpackages/renderer/src/lib/CommandRegistry.tspackages/main/src/preload.tspackages/renderer/src/components/settings/SettingsCategorySidebar.tsxpackages/shared/src/index.tspackages/renderer/src/hooks/useSettings.tspackages/renderer/src/hooks/useKeybindingOverrides.ts
🧠 Learnings (4)
📚 Learning: 2026-03-25T11:00:59.388Z
Learnt from: Cheezeiii365
Repo: Cheezeiii365/aIDE PR: 4
File: packages/renderer/src/components/FileTree/ContextMenu.tsx:16-20
Timestamp: 2026-03-25T11:00:59.388Z
Learning: Since aIDE does not support Windows, path-handling utilities and related code should assume POSIX-style separators (`/`) (e.g., helpers like `dirname`/`basename` that split on `/`). During review, do not require Windows-style backslash (`\\`) support or `path.sep`/cross-platform normalization unless the project explicitly adds Windows support.
Applied to files:
packages/renderer/src/main.tsxpackages/renderer/src/components/DockviewContainer.tsxpackages/main/src/settingsResolver.tspackages/shared/src/commands.tspackages/renderer/src/lib/formatKeybinding.tspackages/renderer/src/components/CommandPalette.tsxpackages/renderer/src/lib/defaultCommands.tspackages/renderer/src/lib/defaultKeybindings.tspackages/renderer/src/components/settings/KeyboardShortcutsTable.tsxpackages/renderer/src/components/AppShell.tsxpackages/renderer/src/components/settings/SettingsHeader.tsxpackages/renderer/src/components/panes/SettingsPane.tsxpackages/renderer/src/components/settings/SettingRow.tsxpackages/main/src/index.tspackages/renderer/src/components/settings/SettingControl.tsxpackages/renderer/src/lib/settingsSchema.tspackages/renderer/src/lib/KeybindingService.tspackages/renderer/src/components/settings/KeybindingRecorder.tsxpackages/renderer/src/components/settings/SettingsContent.tsxpackages/renderer/src/lib/CommandRegistry.tspackages/main/src/preload.tspackages/renderer/src/components/settings/SettingsCategorySidebar.tsxpackages/shared/src/index.tspackages/renderer/src/hooks/useSettings.tspackages/renderer/src/hooks/useKeybindingOverrides.ts
📚 Learning: 2026-03-25T11:11:52.630Z
Learnt from: CR
Repo: Cheezeiii365/aIDE PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T11:11:52.630Z
Learning: Applies to **/*layout*.{ts,tsx} : Use Dockview 5.x for layout and tiling panes
Applied to files:
packages/renderer/src/components/DockviewContainer.tsxpackages/renderer/src/components/AppShell.tsxpackages/renderer/src/components/panes/SettingsPane.tsx
📚 Learning: 2026-03-25T11:11:52.630Z
Learnt from: CR
Repo: Cheezeiii365/aIDE PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T11:11:52.630Z
Learning: Always update IDE_BUILD_PLAN.md when making changes to the codebase
Applied to files:
IDE_BUILD_PLAN.md
📚 Learning: 2026-03-25T11:11:52.630Z
Learnt from: CR
Repo: Cheezeiii365/aIDE PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T11:11:52.630Z
Learning: Applies to **/*.{css,scss,tsx,ts} : Use CSS variable token system with `data-theme` attribute for theming, supporting dark and light mode
Applied to files:
packages/shared/src/index.ts
🔇 Additional comments (18)
packages/renderer/src/lib/formatKeybinding.ts (1)
7-15: LGTM!Clean utility function that correctly normalizes keybinding strings for macOS display. The case-insensitive replacements and collapsing of separators work well for the intended use case.
packages/renderer/src/main.tsx (1)
19-19: LGTM!The CSS import for the settings pane is appropriately placed with other style imports.
packages/renderer/src/components/DockviewContainer.tsx (1)
11-11: LGTM!The
SettingsPaneimport and registration follow the established pattern, and the component key'settingsPane'correctly matches theaddPanelcall inAppShell.tsx.Also applies to: 23-23
packages/main/src/settingsResolver.ts (1)
16-16: LGTM!Exporting
BUILT_IN_DEFAULTSenables the renderer to access built-in defaults through IPC without duplicating the values.packages/renderer/src/lib/defaultKeybindings.ts (2)
7-49: LGTM! Well-organized keybinding rules.The keybinding rules are cleanly categorized, and the "last match wins" documentation correctly reflects the
KeybindingServiceimplementation (per context snippet showingloadKeybindingsconcatenates defaults before overrides).
36-45: Verify that all referenced commands are registered.Several command IDs referenced in lines 36-45 (
terminal.new,view.toggleSidebar,panel.close,markdown.togglePreview,commandPalette.open,quickOpen.open,search.findInFiles,settings.open) require verification. Confirm these commands are registered indefaultCommands.tsor elsewhere (likely inAppShell.tsxviauseCommandhooks).packages/renderer/src/lib/defaultCommands.ts (1)
11-12: LGTM! Clean separation of commands from keybindings.The removal of
keybindingproperties from command definitions aligns with the new architecture wheredefaultKeybindings.tsserves as the single source of truth for key mappings. Command definitions are now properly focused on identity and labeling.Also applies to: 30-31, 50-51, 57-58, 63-64, 69-70, 75-76, 80-81, 86-87, 136-137, 155-156, 163-164
packages/shared/src/commands.ts (1)
5-21: LGTM! Well-designed type separation.The refactored
CommandDefinition(pure identity) and newKeybindingRule(binding + context) interfaces cleanly model the VS Code-style separation. The documentation accurately describes the "last-match-wins" semantics and negative rule convention.packages/renderer/src/components/AppShell.tsx (1)
669-689: LGTM!The
settings.opencommand correctly follows the pattern used for other Dockview panels—activating existing or creating new. Clean implementation.packages/renderer/src/components/settings/SettingRow.tsx (1)
1-36: LGTM!Clean component with well-structured props, proper conditional rendering for modified state, and accessible reset button with title attribute.
packages/main/src/index.ts (1)
519-609: LGTM!The settings and keybinding IPC handlers are well-structured:
- Proper null/undefined handling for value deletion
- Correct error recovery with empty object fallback
- Migration logic correctly handles the legacy
Record<string, string>format- Resolved settings are broadcast after both user and workspace updates
packages/renderer/src/components/settings/SettingsHeader.tsx (1)
1-56: LGTM!Well-implemented header with proper disabled state handling and helpful tooltip for unavailable workspace scope. The
autoFocuson the search input is appropriate UX for a settings pane.packages/renderer/src/styles/settings-pane.css (1)
1-100: LGTM on theming implementation.The stylesheet correctly uses CSS variable tokens throughout for backgrounds, text colors, borders, and accents, ensuring proper dark/light theme support.
packages/renderer/src/components/settings/KeyboardShortcutsTable.tsx (1)
23-68: LGTM on data transformation logic.The rows computation correctly combines keybinding rules with unbound commands, determines override indices for user-sourced rules, and sorts bound commands before unbound ones.
IDE_BUILD_PLAN.md (1)
987-997: LGTM!The build plan accurately documents the completed settings system (two-scope UI, schema-driven settings, keybinding recorder/manager) and correctly updates milestone statuses. Based on learnings: documentation is being kept in sync with codebase changes.
Also applies to: 1210-1210, 1247-1247
packages/renderer/src/components/settings/SettingControl.tsx (1)
9-66: LGTM on control type handling.The switch-based rendering correctly handles all setting types with appropriate HTML controls, proper optional chaining for
enumValues, and sensible defaults for nullish values.packages/renderer/src/components/settings/KeybindingRecorder.tsx (1)
63-65: The recorder cannot intercept the global shortcut handler withstopPropagation().
KeybindingServicealready attached a capture-phasewindowlistener. UsingstopPropagation()in another capture listener cannot prevent that earlier same-target listener from firing first—handlers on the same element execute in registration order, regardless ofstopPropagation(). The global shortcuts will still trigger while recording new keybindings.Also applies to: 107-109
packages/shared/src/index.ts (1)
153-165: Good shared-contract expansion for settings/keybinding IPC.These additions keep channel names and preload API types centralized, which lowers drift risk between main/renderer implementations.
Also applies to: 595-607
Comment on lines
+62
to
+91
| const setValue = useCallback(async (key: string, value: unknown) => { | ||
| if (scope === 'user') { | ||
| // Optimistic update | ||
| setUserSettings((prev) => prev ? { ...prev, [key]: value } : { [key]: value }) | ||
| await window.api.setUserSetting(key, value) | ||
| } else { | ||
| setWorkspaceSettings((prev) => prev ? { ...prev, [key]: value } : { [key]: value }) | ||
| await window.api.setWorkspaceSetting(key, value) | ||
| } | ||
| }, [scope]) | ||
|
|
||
| const resetToDefault = useCallback(async (key: string) => { | ||
| if (scope === 'user') { | ||
| setUserSettings((prev) => { | ||
| if (!prev) return prev | ||
| const next = { ...prev } | ||
| delete (next as Record<string, unknown>)[key] | ||
| return next | ||
| }) | ||
| await window.api.setUserSetting(key, undefined) | ||
| } else { | ||
| setWorkspaceSettings((prev) => { | ||
| if (!prev) return prev | ||
| const next = { ...prev } | ||
| delete (next as Record<string, unknown>)[key] | ||
| return next | ||
| }) | ||
| await window.api.setWorkspaceSetting(key, undefined) | ||
| } | ||
| }, [scope]) |
Contributor
There was a problem hiding this comment.
Optimistic writes need failure reconciliation.
When setUserSetting / setWorkspaceSetting fails, local state stays mutated and diverges from persisted state.
Proposed fix
const setValue = useCallback(async (key: string, value: unknown) => {
- if (scope === 'user') {
- // Optimistic update
- setUserSettings((prev) => prev ? { ...prev, [key]: value } : { [key]: value })
- await window.api.setUserSetting(key, value)
- } else {
- setWorkspaceSettings((prev) => prev ? { ...prev, [key]: value } : { [key]: value })
- await window.api.setWorkspaceSetting(key, value)
- }
+ if (scope === 'user') {
+ await window.api.setUserSetting(key, value)
+ setUserSettings((prev) => (prev ? { ...prev, [key]: value } : { [key]: value }))
+ } else {
+ await window.api.setWorkspaceSetting(key, value)
+ setWorkspaceSettings((prev) => (prev ? { ...prev, [key]: value } : { [key]: value }))
+ }
}, [scope])
const resetToDefault = useCallback(async (key: string) => {
if (scope === 'user') {
- setUserSettings((prev) => {
- if (!prev) return prev
- const next = { ...prev }
- delete (next as Record<string, unknown>)[key]
- return next
- })
await window.api.setUserSetting(key, undefined)
+ setUserSettings((prev) => {
+ if (!prev) return prev
+ const next = { ...prev }
+ delete (next as Record<string, unknown>)[key]
+ return next
+ })
} else {
- setWorkspaceSettings((prev) => {
- if (!prev) return prev
- const next = { ...prev }
- delete (next as Record<string, unknown>)[key]
- return next
- })
await window.api.setWorkspaceSetting(key, undefined)
+ setWorkspaceSettings((prev) => {
+ if (!prev) return prev
+ const next = { ...prev }
+ delete (next as Record<string, unknown>)[key]
+ return next
+ })
}
}, [scope])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const setValue = useCallback(async (key: string, value: unknown) => { | |
| if (scope === 'user') { | |
| // Optimistic update | |
| setUserSettings((prev) => prev ? { ...prev, [key]: value } : { [key]: value }) | |
| await window.api.setUserSetting(key, value) | |
| } else { | |
| setWorkspaceSettings((prev) => prev ? { ...prev, [key]: value } : { [key]: value }) | |
| await window.api.setWorkspaceSetting(key, value) | |
| } | |
| }, [scope]) | |
| const resetToDefault = useCallback(async (key: string) => { | |
| if (scope === 'user') { | |
| setUserSettings((prev) => { | |
| if (!prev) return prev | |
| const next = { ...prev } | |
| delete (next as Record<string, unknown>)[key] | |
| return next | |
| }) | |
| await window.api.setUserSetting(key, undefined) | |
| } else { | |
| setWorkspaceSettings((prev) => { | |
| if (!prev) return prev | |
| const next = { ...prev } | |
| delete (next as Record<string, unknown>)[key] | |
| return next | |
| }) | |
| await window.api.setWorkspaceSetting(key, undefined) | |
| } | |
| }, [scope]) | |
| const setValue = useCallback(async (key: string, value: unknown) => { | |
| if (scope === 'user') { | |
| await window.api.setUserSetting(key, value) | |
| setUserSettings((prev) => (prev ? { ...prev, [key]: value } : { [key]: value })) | |
| } else { | |
| await window.api.setWorkspaceSetting(key, value) | |
| setWorkspaceSettings((prev) => (prev ? { ...prev, [key]: value } : { [key]: value })) | |
| } | |
| }, [scope]) | |
| const resetToDefault = useCallback(async (key: string) => { | |
| if (scope === 'user') { | |
| await window.api.setUserSetting(key, undefined) | |
| setUserSettings((prev) => { | |
| if (!prev) return prev | |
| const next = { ...prev } | |
| delete (next as Record<string, unknown>)[key] | |
| return next | |
| }) | |
| } else { | |
| await window.api.setWorkspaceSetting(key, undefined) | |
| setWorkspaceSettings((prev) => { | |
| if (!prev) return prev | |
| const next = { ...prev } | |
| delete (next as Record<string, unknown>)[key] | |
| return next | |
| }) | |
| } | |
| }, [scope]) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/renderer/src/hooks/useSettings.ts` around lines 62 - 91, The
optimistic updates in setValue and resetToDefault mutate local state via
setUserSettings/setWorkspaceSettings before awaiting
window.api.setUserSetting/setWorkspaceSetting, but there is no reconciliation on
failure; wrap the API calls in try/catch and on error revert the optimistic
state to the previous value (capture prev inside the updater or store a snapshot
before updating), restore that snapshot in the catch block, and propagate or
surface the error; update both setValue and resetToDefault to follow this
pattern so local state stays consistent with persisted state.
Comment on lines
+135
to
+157
| export function loadKeybindings( | ||
| defaults: KeybindingRule[], | ||
| userOverrides: KeybindingRule[] = [], | ||
| ): void { | ||
| ensureListener() | ||
|
|
||
| const resolved: ResolvedRule[] = [ | ||
| ...defaults.map((rule) => ({ | ||
| rule, | ||
| parts: parse(rule.key), | ||
| source: 'default' as RuleSource, | ||
| suppressed: false, | ||
| })), | ||
| ...userOverrides.map((rule) => ({ | ||
| rule, | ||
| parts: parse(rule.key), | ||
| source: 'user' as RuleSource, | ||
| suppressed: false, | ||
| })), | ||
| ] | ||
|
|
||
| processRemovals(resolved) | ||
| rules = resolved |
Contributor
There was a problem hiding this comment.
Reloading overrides drops component-scoped bindings.
useKeybinding() appends live rules into the shared rules array, but loadKeybindings() replaces that array wholesale. Any mounted lifecycle-scoped shortcut disappears the next time defaults/overrides are reloaded.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/renderer/src/lib/KeybindingService.ts` around lines 135 - 157,
loadKeybindings currently overwrites the shared rules array which drops
component-scoped/live rules added by useKeybinding; change loadKeybindings to
merge resolved rules with existing rules instead of replacing them: filter the
existing global rules (the variable rules) to keep any entries with the
component-scoped/source marker used by useKeybinding (e.g., source ===
'component' or a live flag), merge them with the newly computed resolved array
while deduplicating by rule.key (or rule id) so defaults/user overrides still
supersede, then run processRemovals and assign the merged array back to rules;
update logic around parse(rule.key) and processRemovals to operate on the merged
set to preserve lifecycle-scoped shortcuts.
Comment on lines
+126
to
+127
| export function getCategoriesWithSettings(): Set<string> { | ||
| return new Set(SETTINGS_DESCRIPTORS.map((d) => d.category)) |
Contributor
There was a problem hiding this comment.
Include non-descriptor pages in categoriesWithSettings.
This helper only returns descriptor-backed categories, but the settings UI also renders special sections without descriptors. Those pages will be styled as empty in the sidebar even though they have content.
Suggested fix
+const NON_DESCRIPTOR_CATEGORIES_WITH_CONTENT = [
+ 'workbench.appearance',
+ 'keyboardShortcuts',
+]
+
/** Get all category IDs that have at least one setting descriptor */
export function getCategoriesWithSettings(): Set<string> {
- return new Set(SETTINGS_DESCRIPTORS.map((d) => d.category))
+ return new Set([
+ ...SETTINGS_DESCRIPTORS.map((d) => d.category),
+ ...NON_DESCRIPTOR_CATEGORIES_WITH_CONTENT,
+ ])
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function getCategoriesWithSettings(): Set<string> { | |
| return new Set(SETTINGS_DESCRIPTORS.map((d) => d.category)) | |
| const NON_DESCRIPTOR_CATEGORIES_WITH_CONTENT = [ | |
| 'workbench.appearance', | |
| 'keyboardShortcuts', | |
| ] | |
| /** Get all category IDs that have at least one setting descriptor */ | |
| export function getCategoriesWithSettings(): Set<string> { | |
| return new Set([ | |
| ...SETTINGS_DESCRIPTORS.map((d) => d.category), | |
| ...NON_DESCRIPTOR_CATEGORIES_WITH_CONTENT, | |
| ]) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/renderer/src/lib/settingsSchema.ts` around lines 126 - 127,
getCategoriesWithSettings currently builds categories only from
SETTINGS_DESCRIPTORS, omitting special non-descriptor pages; update the function
to also include category values from your non-descriptor pages collection (e.g.,
SETTINGS_PAGES, SETTINGS_NON_DESCRIPTOR_PAGES, or whatever array/object holds
the special pages) by iterating that collection and adding each page.category
into the same Set before returning it so the sidebar treats those sections as
non-empty; keep getCategoriesWithSettings and SETTINGS_DESCRIPTORS as the
primary anchors when making the change.
Replace hardcoded #e5534b with var(--text-error) so the conflict message adapts to dark/light theme switches. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Recording state previously used only commandId, causing all rows sharing the same command to show the recorder simultaneously. Now uses a composite key (commandId-key-idx) so each row is independently recordable. Also preserves the when clause in addOverride. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove redundant userDefaults layer in resolveSettings since appDefaults already incorporates user defaults. Add error handling for keybinding overrides loading to fall back to defaults on failure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
addOverride and removeOverride now fetch current state from the main process instead of closing over the render-time overrides array. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Cmd and Ctrl were conflated into a single `meta` flag, making it impossible to bind Ctrl+key shortcuts on macOS independently from Cmd+key. Additionally, key aliases emitted by the recorder (Space, Up, Down, etc.) did not match browser KeyboardEvent.key values, so recorded shortcuts with those keys silently never fired. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Mar 27, 2026
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
1 participant
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
KeybindingService, keybinding recorder, and keyboard shortcuts tableTest plan
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com
Summary by CodeRabbit
New Features
Refactor