mirror of
https://github.com/earendil-works/pi.git
synced 2026-06-18 15:54:04 +08:00
feat(coding-agent): make scoped models and tree filter shortcuts configurable (#3343)
- Add app.models.{save,enableAll,clearAll,toggleProvider,reorderUp,reorderDown}
- Add app.tree.filter.{default,noTools,userOnly,labeledOnly,all,cycleForward,cycleBackward}
- Migrate scoped-models-selector cancel to tui.select.cancel
- Prefer reserved actions on key collision in extension shortcut conflict check
This commit is contained in:
Marek Pazik
committed by
GitHub
Unverified
GitHub
parent
d554409b1f
commit
d4e2e563ae
@@ -130,6 +130,26 @@ Modifier combinations: `ctrl+shift+x`, `alt+ctrl+x`, `ctrl+shift+alt+x`, `ctrl+1
|
||||
| `app.tree.unfoldOrDown` | `ctrl+right`, `alt+right` | Unfold current branch segment, or jump to the next segment start or branch end |
|
||||
| `app.tree.editLabel` | `shift+l` | Edit the label on the selected tree node |
|
||||
| `app.tree.toggleLabelTimestamp` | `shift+t` | Toggle label timestamps in the tree |
|
||||
| `app.tree.filter.default` | `ctrl+d` | Set tree filter to default view |
|
||||
| `app.tree.filter.noTools` | `ctrl+t` | Toggle tree filter that hides tool results |
|
||||
| `app.tree.filter.userOnly` | `ctrl+u` | Toggle tree filter that shows only user messages |
|
||||
| `app.tree.filter.labeledOnly` | `ctrl+l` | Toggle tree filter that shows only labeled entries |
|
||||
| `app.tree.filter.all` | `ctrl+a` | Toggle tree filter that shows all entries |
|
||||
| `app.tree.filter.cycleForward` | `ctrl+o` | Cycle tree filter forward |
|
||||
| `app.tree.filter.cycleBackward` | `shift+ctrl+o` | Cycle tree filter backward |
|
||||
|
||||
### Scoped Models Selector
|
||||
|
||||
Used inside the scoped models selector (opened via `/scoped-models`).
|
||||
|
||||
| Keybinding id | Default | Description |
|
||||
|--------|---------|-------------|
|
||||
| `app.models.save` | `ctrl+s` | Save current model selection to settings |
|
||||
| `app.models.enableAll` | `ctrl+a` | Enable all models (or all matching the current search) |
|
||||
| `app.models.clearAll` | `ctrl+x` | Clear all models (or all matching the current search) |
|
||||
| `app.models.toggleProvider` | `ctrl+p` | Toggle all models for the current provider |
|
||||
| `app.models.reorderUp` | `alt+up` | Move the selected model up in the cycle order |
|
||||
| `app.models.reorderDown` | `alt+down` | Move the selected model down in the cycle order |
|
||||
|
||||
## Custom Configuration
|
||||
|
||||
|
||||
@@ -84,6 +84,10 @@ const buildBuiltinKeybindings = (resolvedKeybindings: KeybindingsConfig): BuiltI
|
||||
const restrictOverride = (RESERVED_KEYBINDINGS_FOR_EXTENSION_CONFLICTS as readonly string[]).includes(keybinding);
|
||||
for (const key of keyList) {
|
||||
const normalizedKey = key.toLowerCase() as KeyId;
|
||||
// If multiple actions bind the same key, the reserved action wins so extensions
|
||||
// remain blocked by reserved shortcuts regardless of iteration order.
|
||||
const existing = builtinKeybindings[normalizedKey];
|
||||
if (existing?.restrictOverride && !restrictOverride) continue;
|
||||
builtinKeybindings[normalizedKey] = {
|
||||
keybinding,
|
||||
restrictOverride,
|
||||
|
||||
@@ -39,6 +39,19 @@ export interface AppKeybindings {
|
||||
"app.session.rename": true;
|
||||
"app.session.delete": true;
|
||||
"app.session.deleteNoninvasive": true;
|
||||
"app.models.save": true;
|
||||
"app.models.enableAll": true;
|
||||
"app.models.clearAll": true;
|
||||
"app.models.toggleProvider": true;
|
||||
"app.models.reorderUp": true;
|
||||
"app.models.reorderDown": true;
|
||||
"app.tree.filter.default": true;
|
||||
"app.tree.filter.noTools": true;
|
||||
"app.tree.filter.userOnly": true;
|
||||
"app.tree.filter.labeledOnly": true;
|
||||
"app.tree.filter.all": true;
|
||||
"app.tree.filter.cycleForward": true;
|
||||
"app.tree.filter.cycleBackward": true;
|
||||
}
|
||||
|
||||
export type AppKeybinding = keyof AppKeybindings;
|
||||
@@ -134,6 +147,58 @@ export const KEYBINDINGS = {
|
||||
defaultKeys: "ctrl+backspace",
|
||||
description: "Delete session when query is empty",
|
||||
},
|
||||
"app.models.save": {
|
||||
defaultKeys: "ctrl+s",
|
||||
description: "Save model selection",
|
||||
},
|
||||
"app.models.enableAll": {
|
||||
defaultKeys: "ctrl+a",
|
||||
description: "Enable all models",
|
||||
},
|
||||
"app.models.clearAll": {
|
||||
defaultKeys: "ctrl+x",
|
||||
description: "Clear all models",
|
||||
},
|
||||
"app.models.toggleProvider": {
|
||||
defaultKeys: "ctrl+p",
|
||||
description: "Toggle all models for provider",
|
||||
},
|
||||
"app.models.reorderUp": {
|
||||
defaultKeys: "alt+up",
|
||||
description: "Move model up in order",
|
||||
},
|
||||
"app.models.reorderDown": {
|
||||
defaultKeys: "alt+down",
|
||||
description: "Move model down in order",
|
||||
},
|
||||
"app.tree.filter.default": {
|
||||
defaultKeys: "ctrl+d",
|
||||
description: "Tree filter: default view",
|
||||
},
|
||||
"app.tree.filter.noTools": {
|
||||
defaultKeys: "ctrl+t",
|
||||
description: "Tree filter: hide tool results",
|
||||
},
|
||||
"app.tree.filter.userOnly": {
|
||||
defaultKeys: "ctrl+u",
|
||||
description: "Tree filter: user messages only",
|
||||
},
|
||||
"app.tree.filter.labeledOnly": {
|
||||
defaultKeys: "ctrl+l",
|
||||
description: "Tree filter: labeled entries only",
|
||||
},
|
||||
"app.tree.filter.all": {
|
||||
defaultKeys: "ctrl+a",
|
||||
description: "Tree filter: show all entries",
|
||||
},
|
||||
"app.tree.filter.cycleForward": {
|
||||
defaultKeys: "ctrl+o",
|
||||
description: "Tree filter: cycle forward",
|
||||
},
|
||||
"app.tree.filter.cycleBackward": {
|
||||
defaultKeys: "shift+ctrl+o",
|
||||
description: "Tree filter: cycle backward",
|
||||
},
|
||||
} as const satisfies KeybindingDefinitions;
|
||||
|
||||
const KEYBINDING_NAME_MIGRATIONS = {
|
||||
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
} from "@mariozechner/pi-tui";
|
||||
import { theme } from "../theme/theme.js";
|
||||
import { DynamicBorder } from "./dynamic-border.js";
|
||||
import { keyText } from "./keybinding-hints.js";
|
||||
|
||||
// EnabledIds: null = all enabled (no filter), string[] = explicit ordered list
|
||||
type EnabledIds = string[] | null;
|
||||
@@ -126,7 +127,9 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
|
||||
this.addChild(new DynamicBorder());
|
||||
this.addChild(new Spacer(1));
|
||||
this.addChild(new Text(theme.fg("accent", theme.bold("Model Configuration")), 0, 0));
|
||||
this.addChild(new Text(theme.fg("muted", "Session-only. Ctrl+S to save to settings."), 0, 0));
|
||||
this.addChild(
|
||||
new Text(theme.fg("muted", `Session-only. ${keyText("app.models.save")} to save to settings.`), 0, 0),
|
||||
);
|
||||
this.addChild(new Spacer(1));
|
||||
|
||||
// Search input
|
||||
@@ -162,7 +165,15 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
|
||||
const enabledCount = this.enabledIds?.length ?? this.allIds.length;
|
||||
const allEnabled = this.enabledIds === null;
|
||||
const countText = allEnabled ? "all enabled" : `${enabledCount}/${this.allIds.length} enabled`;
|
||||
const parts = ["Enter toggle", "^A all", "^X clear", "^P provider", "Alt+↑↓ reorder", "^S save", countText];
|
||||
const parts = [
|
||||
`${keyText("tui.select.confirm")} toggle`,
|
||||
`${keyText("app.models.enableAll")} all`,
|
||||
`${keyText("app.models.clearAll")} clear`,
|
||||
`${keyText("app.models.toggleProvider")} provider`,
|
||||
`${keyText("app.models.reorderUp")}/${keyText("app.models.reorderDown")} reorder`,
|
||||
`${keyText("app.models.save")} save`,
|
||||
countText,
|
||||
];
|
||||
return this.isDirty
|
||||
? theme.fg("dim", ` ${parts.join(" · ")} `) + theme.fg("warning", "(unsaved)")
|
||||
: theme.fg("dim", ` ${parts.join(" · ")}`);
|
||||
@@ -237,12 +248,14 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
|
||||
return;
|
||||
}
|
||||
|
||||
// Alt+Up/Down - Reorder enabled models
|
||||
if (matchesKey(data, Key.alt("up")) || matchesKey(data, Key.alt("down"))) {
|
||||
// Reorder enabled models
|
||||
const reorderUp = kb.matches(data, "app.models.reorderUp");
|
||||
const reorderDown = kb.matches(data, "app.models.reorderDown");
|
||||
if (reorderUp || reorderDown) {
|
||||
if (this.enabledIds === null) return;
|
||||
const item = this.filteredItems[this.selectedIndex];
|
||||
if (item && isEnabled(this.enabledIds, item.fullId)) {
|
||||
const delta = matchesKey(data, Key.alt("up")) ? -1 : 1;
|
||||
const delta = reorderUp ? -1 : 1;
|
||||
const currentIndex = this.enabledIds.indexOf(item.fullId);
|
||||
const newIndex = currentIndex + delta;
|
||||
// Only move if within bounds
|
||||
@@ -258,7 +271,7 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
|
||||
}
|
||||
|
||||
// Toggle on Enter
|
||||
if (matchesKey(data, Key.enter)) {
|
||||
if (kb.matches(data, "tui.select.confirm")) {
|
||||
const item = this.filteredItems[this.selectedIndex];
|
||||
if (item) {
|
||||
this.enabledIds = toggle(this.enabledIds, item.fullId);
|
||||
@@ -269,8 +282,8 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
|
||||
return;
|
||||
}
|
||||
|
||||
// Ctrl+A - Enable all (filtered if search active, otherwise all)
|
||||
if (matchesKey(data, Key.ctrl("a"))) {
|
||||
// Enable all (filtered if search active, otherwise all)
|
||||
if (kb.matches(data, "app.models.enableAll")) {
|
||||
const targetIds = this.searchInput.getValue() ? this.filteredItems.map((i) => i.fullId) : undefined;
|
||||
this.enabledIds = enableAll(this.enabledIds, this.allIds, targetIds);
|
||||
this.isDirty = true;
|
||||
@@ -279,8 +292,8 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
|
||||
return;
|
||||
}
|
||||
|
||||
// Ctrl+X - Clear all (filtered if search active, otherwise all)
|
||||
if (matchesKey(data, Key.ctrl("x"))) {
|
||||
// Clear all (filtered if search active, otherwise all)
|
||||
if (kb.matches(data, "app.models.clearAll")) {
|
||||
const targetIds = this.searchInput.getValue() ? this.filteredItems.map((i) => i.fullId) : undefined;
|
||||
this.enabledIds = clearAll(this.enabledIds, this.allIds, targetIds);
|
||||
this.isDirty = true;
|
||||
@@ -289,8 +302,8 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
|
||||
return;
|
||||
}
|
||||
|
||||
// Ctrl+P - Toggle provider of current item
|
||||
if (matchesKey(data, Key.ctrl("p"))) {
|
||||
// Toggle provider of current item
|
||||
if (kb.matches(data, "app.models.toggleProvider")) {
|
||||
const item = this.filteredItems[this.selectedIndex];
|
||||
if (item) {
|
||||
const provider = item.model.provider;
|
||||
@@ -306,8 +319,8 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
|
||||
return;
|
||||
}
|
||||
|
||||
// Ctrl+S - Save/persist to settings
|
||||
if (matchesKey(data, Key.ctrl("s"))) {
|
||||
// Save/persist to settings
|
||||
if (kb.matches(data, "app.models.save")) {
|
||||
this.callbacks.onPersist(this.enabledIds === null ? null : [...this.enabledIds]);
|
||||
this.isDirty = false;
|
||||
this.footerText.setText(this.getFooterText());
|
||||
|
||||
@@ -4,7 +4,6 @@ import {
|
||||
type Focusable,
|
||||
getKeybindings,
|
||||
Input,
|
||||
matchesKey,
|
||||
Spacer,
|
||||
Text,
|
||||
TruncatedText,
|
||||
@@ -938,39 +937,39 @@ class TreeList implements Component {
|
||||
} else {
|
||||
this.onCancel?.();
|
||||
}
|
||||
} else if (matchesKey(keyData, "ctrl+d")) {
|
||||
} else if (kb.matches(keyData, "app.tree.filter.default")) {
|
||||
// Direct filter: default
|
||||
this.filterMode = "default";
|
||||
this.foldedNodes.clear();
|
||||
this.applyFilter();
|
||||
} else if (matchesKey(keyData, "ctrl+t")) {
|
||||
} else if (kb.matches(keyData, "app.tree.filter.noTools")) {
|
||||
// Toggle filter: no-tools ↔ default
|
||||
this.filterMode = this.filterMode === "no-tools" ? "default" : "no-tools";
|
||||
this.foldedNodes.clear();
|
||||
this.applyFilter();
|
||||
} else if (matchesKey(keyData, "ctrl+u")) {
|
||||
} else if (kb.matches(keyData, "app.tree.filter.userOnly")) {
|
||||
// Toggle filter: user-only ↔ default
|
||||
this.filterMode = this.filterMode === "user-only" ? "default" : "user-only";
|
||||
this.foldedNodes.clear();
|
||||
this.applyFilter();
|
||||
} else if (matchesKey(keyData, "ctrl+l")) {
|
||||
} else if (kb.matches(keyData, "app.tree.filter.labeledOnly")) {
|
||||
// Toggle filter: labeled-only ↔ default
|
||||
this.filterMode = this.filterMode === "labeled-only" ? "default" : "labeled-only";
|
||||
this.foldedNodes.clear();
|
||||
this.applyFilter();
|
||||
} else if (matchesKey(keyData, "ctrl+a")) {
|
||||
} else if (kb.matches(keyData, "app.tree.filter.all")) {
|
||||
// Toggle filter: all ↔ default
|
||||
this.filterMode = this.filterMode === "all" ? "default" : "all";
|
||||
this.foldedNodes.clear();
|
||||
this.applyFilter();
|
||||
} else if (matchesKey(keyData, "shift+ctrl+o")) {
|
||||
} else if (kb.matches(keyData, "app.tree.filter.cycleBackward")) {
|
||||
// Cycle filter backwards
|
||||
const modes: FilterMode[] = ["default", "no-tools", "user-only", "labeled-only", "all"];
|
||||
const currentIndex = modes.indexOf(this.filterMode);
|
||||
this.filterMode = modes[(currentIndex - 1 + modes.length) % modes.length];
|
||||
this.foldedNodes.clear();
|
||||
this.applyFilter();
|
||||
} else if (matchesKey(keyData, "ctrl+o")) {
|
||||
} else if (kb.matches(keyData, "app.tree.filter.cycleForward")) {
|
||||
// Cycle filter forwards: default → no-tools → user-only → labeled-only → all → default
|
||||
const modes: FilterMode[] = ["default", "no-tools", "user-only", "labeled-only", "all"];
|
||||
const currentIndex = modes.indexOf(this.filterMode);
|
||||
@@ -1178,11 +1177,19 @@ export class TreeSelectorComponent extends Container implements Focusable {
|
||||
this.addChild(new Spacer(1));
|
||||
this.addChild(new DynamicBorder());
|
||||
this.addChild(new Text(theme.bold(" Session Tree"), 1, 0));
|
||||
const filterKeys = [
|
||||
keyText("app.tree.filter.default"),
|
||||
keyText("app.tree.filter.noTools"),
|
||||
keyText("app.tree.filter.userOnly"),
|
||||
keyText("app.tree.filter.labeledOnly"),
|
||||
keyText("app.tree.filter.all"),
|
||||
].join("/");
|
||||
const cycleKeys = `${keyText("app.tree.filter.cycleForward")}/${keyText("app.tree.filter.cycleBackward")}`;
|
||||
this.addChild(
|
||||
new TruncatedText(
|
||||
theme.fg(
|
||||
"muted",
|
||||
` ↑/↓: move. ←/→: page. ^←/^→ or Alt+←/Alt+→: fold/branch. ${keyText("app.tree.editLabel")}: label. ^D/^T/^U/^L/^A: filters (^O/⇧^O cycle). ${keyText("app.tree.toggleLabelTimestamp")}: label time`,
|
||||
` ↑/↓: move. ←/→: page. ^←/^→ or Alt+←/Alt+→: fold/branch. ${keyText("app.tree.editLabel")}: label. ${filterKeys}: filters (${cycleKeys} cycle). ${keyText("app.tree.toggleLabelTimestamp")}: label time`,
|
||||
),
|
||||
0,
|
||||
0,
|
||||
|
||||
@@ -180,6 +180,29 @@ describe("ExtensionRunner", () => {
|
||||
warnSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("blocks shortcuts when reserved key is also bound to non-reserved actions", async () => {
|
||||
const extCode = `
|
||||
export default function(pi) {
|
||||
pi.registerShortcut("ctrl+p", {
|
||||
description: "Conflicts with shared reserved default",
|
||||
handler: async () => {},
|
||||
});
|
||||
}
|
||||
`;
|
||||
fs.writeFileSync(path.join(extensionsDir, "shared-reserved.ts"), extCode);
|
||||
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
|
||||
const result = await discoverAndLoadExtensions([], tempDir, tempDir);
|
||||
const runner = new ExtensionRunner(result.extensions, result.runtime, tempDir, sessionManager, modelRegistry);
|
||||
const shortcuts = runner.getShortcuts(defaultKeybindings);
|
||||
|
||||
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("conflicts with built-in"));
|
||||
expect(shortcuts.has("ctrl+p")).toBe(false);
|
||||
|
||||
warnSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("blocks shortcuts when reserved action has multiple keys", async () => {
|
||||
const extCode = `
|
||||
export default function(pi) {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import type { TUI } from "@mariozechner/pi-tui";
|
||||
import { setKeybindings, type TUI } from "@mariozechner/pi-tui";
|
||||
import stripAnsi from "strip-ansi";
|
||||
import { afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import { afterEach, beforeAll, beforeEach, describe, expect, it } from "vitest";
|
||||
import { KeybindingsManager } from "../../../src/core/keybindings.js";
|
||||
import { ModelSelectorComponent } from "../../../src/modes/interactive/components/model-selector.js";
|
||||
import { ScopedModelsSelectorComponent } from "../../../src/modes/interactive/components/scoped-models-selector.js";
|
||||
import { initTheme } from "../../../src/modes/interactive/theme/theme.js";
|
||||
@@ -23,6 +24,11 @@ describe("issue #3217 scoped model ordering", () => {
|
||||
initTheme("dark");
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
// Ensure test isolation: keybindings are a global singleton
|
||||
setKeybindings(new KeybindingsManager());
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
while (harnesses.length > 0) {
|
||||
harnesses.pop()?.cleanup();
|
||||
|
||||
Reference in New Issue
Block a user