feat(coding-agent): migrated keybindings config to YAML with JSON migration
- Changed canonical config path from `keybindings.json` to `keybindings.yml`. - Added automatic migration of legacy JSON to YAML on first load. - Retained read support for `keybindings.yaml` without promoting it to canonical. - Added tests for yml, yaml, and JSON migration scenarios.
This commit is contained in:
@@ -12,6 +12,7 @@
|
||||
- Changed `search` output to preserve full virtual and internal URL paths in grouped results and `details.files` instead of collapsing them to file basenames
|
||||
- Changed `/omfg` to run up to three generation attempts with validation feedback and only prompt saving when no draft matches assistant history
|
||||
- Changed `/omfg` to show a live draft panel with generation/validation/saving status and allow canceling an active rule request with `Esc`
|
||||
- Changed keybindings config to use `~/.omp/agent/keybindings.yml`, with automatic migration from legacy `keybindings.json` and continued support for `keybindings.yaml`.
|
||||
|
||||
### Fixed
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { existsSync, readFileSync, writeFileSync } from "node:fs";
|
||||
import * as fs from "node:fs";
|
||||
import * as path from "node:path";
|
||||
import {
|
||||
type Keybinding,
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
KeybindingsManager as TuiKeybindingsManager,
|
||||
} from "@oh-my-pi/pi-tui";
|
||||
import { getAgentDir, isEnoent, logger } from "@oh-my-pi/pi-utils";
|
||||
import { YAML } from "bun";
|
||||
|
||||
/**
|
||||
* Application-level keybindings (coding agent specific).
|
||||
@@ -330,17 +331,29 @@ function orderKeybindingsConfig(config: KeybindingsConfig): KeybindingsConfig {
|
||||
return ordered;
|
||||
}
|
||||
|
||||
const KEYBINDINGS_YML = "keybindings.yml";
|
||||
const KEYBINDINGS_YAML = "keybindings.yaml";
|
||||
const LEGACY_KEYBINDINGS_JSON = "keybindings.json";
|
||||
|
||||
interface KeybindingsConfigPaths {
|
||||
readPath: string;
|
||||
writeBackPath: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Load raw config from a file synchronously.
|
||||
* Returns parsed JSON or null if file doesn't exist or is invalid.
|
||||
* Returns parsed JSON/YAML or null if file doesn't exist or is invalid.
|
||||
*/
|
||||
function loadRawConfig(filePath: string): unknown {
|
||||
try {
|
||||
if (!existsSync(filePath)) {
|
||||
return null;
|
||||
const content = fs.readFileSync(filePath, "utf-8");
|
||||
if (filePath.endsWith(".json")) {
|
||||
return JSON.parse(content);
|
||||
}
|
||||
const content = readFileSync(filePath, "utf-8");
|
||||
return JSON.parse(content);
|
||||
if (filePath.endsWith(".yml") || filePath.endsWith(".yaml")) {
|
||||
return YAML.parse(content);
|
||||
}
|
||||
throw new Error(`Unsupported keybindings config extension: ${filePath}`);
|
||||
} catch (error) {
|
||||
if (isEnoent(error)) {
|
||||
return null;
|
||||
@@ -350,34 +363,67 @@ function loadRawConfig(filePath: string): unknown {
|
||||
}
|
||||
}
|
||||
|
||||
function writeKeybindingsConfig(filePath: string, config: KeybindingsConfig): boolean {
|
||||
try {
|
||||
fs.writeFileSync(filePath, YAML.stringify(config, null, 2), "utf-8");
|
||||
logger.debug("Migrated keybindings config", { path: filePath });
|
||||
return true;
|
||||
} catch (error) {
|
||||
logger.warn("Failed to write migrated keybindings config", { path: filePath, error: String(error) });
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
function resolveKeybindingsConfigPaths(agentDir: string): KeybindingsConfigPaths {
|
||||
const ymlPath = path.join(agentDir, KEYBINDINGS_YML);
|
||||
if (fs.existsSync(ymlPath)) {
|
||||
return { readPath: ymlPath, writeBackPath: ymlPath };
|
||||
}
|
||||
|
||||
const yamlPath = path.join(agentDir, KEYBINDINGS_YAML);
|
||||
if (fs.existsSync(yamlPath)) {
|
||||
return { readPath: yamlPath, writeBackPath: yamlPath };
|
||||
}
|
||||
|
||||
const jsonPath = path.join(agentDir, LEGACY_KEYBINDINGS_JSON);
|
||||
if (fs.existsSync(jsonPath)) {
|
||||
return { readPath: jsonPath, writeBackPath: ymlPath };
|
||||
}
|
||||
|
||||
return { readPath: ymlPath, writeBackPath: ymlPath };
|
||||
}
|
||||
|
||||
/**
|
||||
* Migrate keybindings config file from old format to new.
|
||||
* Reads from agentDir/keybindings.json, migrates old names, and writes back.
|
||||
* Load and migrate keybindings config.
|
||||
* Legacy JSON is read for compatibility, but successful write-back goes to YAML.
|
||||
*/
|
||||
function loadKeybindingsConfig(filePath: string, writeBack: boolean): KeybindingsConfig {
|
||||
function loadKeybindingsConfig(
|
||||
filePath: string,
|
||||
writeBackPath: string | undefined,
|
||||
): {
|
||||
config: KeybindingsConfig;
|
||||
persistedPath: string;
|
||||
} {
|
||||
const rawConfig = loadRawConfig(filePath);
|
||||
|
||||
if (rawConfig === null) {
|
||||
return {};
|
||||
return { config: {}, persistedPath: filePath };
|
||||
}
|
||||
|
||||
const { config: migratedConfig, migrated } = migrateKeybindingNames(rawConfig);
|
||||
if (writeBack && migrated) {
|
||||
const shouldWriteBack = writeBackPath !== undefined && (migrated || writeBackPath !== filePath);
|
||||
if (shouldWriteBack) {
|
||||
const ordered = orderKeybindingsConfig(migratedConfig);
|
||||
try {
|
||||
writeFileSync(filePath, `${JSON.stringify(ordered, null, 2)}\n`, "utf-8");
|
||||
logger.debug("Migrated keybindings config", { path: filePath });
|
||||
} catch (error) {
|
||||
logger.warn("Failed to write migrated keybindings config", { path: filePath, error: String(error) });
|
||||
}
|
||||
const persistedPath = writeKeybindingsConfig(writeBackPath, ordered) ? writeBackPath : filePath;
|
||||
return { config: migratedConfig, persistedPath };
|
||||
}
|
||||
|
||||
return migratedConfig;
|
||||
return { config: migratedConfig, persistedPath: filePath };
|
||||
}
|
||||
|
||||
function migrateKeybindingsConfigFile(agentDir: string): void {
|
||||
const configPath = path.join(agentDir, "keybindings.json");
|
||||
loadKeybindingsConfig(configPath, true);
|
||||
const { readPath, writeBackPath } = resolveKeybindingsConfigPaths(agentDir);
|
||||
loadKeybindingsConfig(readPath, writeBackPath);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -393,12 +439,13 @@ export class KeybindingsManager extends TuiKeybindingsManager {
|
||||
}
|
||||
|
||||
/**
|
||||
* Create from config file at agentDir/keybindings.json.
|
||||
* Create from config file at agentDir/keybindings.yml.
|
||||
* Legacy keybindings.json is migrated to keybindings.yml on load.
|
||||
*/
|
||||
static create(agentDir: string = getAgentDir()): KeybindingsManager {
|
||||
const configPath = path.join(agentDir, "keybindings.json");
|
||||
const userBindings = KeybindingsManager.#loadFromFile(configPath);
|
||||
const manager = new KeybindingsManager(userBindings, configPath);
|
||||
const { readPath, writeBackPath } = resolveKeybindingsConfigPaths(agentDir);
|
||||
const { config: userBindings, persistedPath } = KeybindingsManager.#loadFromFile(readPath, writeBackPath);
|
||||
const manager = new KeybindingsManager(userBindings, persistedPath);
|
||||
// Set globally so getKeybindings() returns this manager
|
||||
setKeybindings(manager);
|
||||
return manager;
|
||||
@@ -416,7 +463,8 @@ export class KeybindingsManager extends TuiKeybindingsManager {
|
||||
*/
|
||||
reload(): void {
|
||||
if (!this.#configPath) return;
|
||||
this.setUserBindings(KeybindingsManager.#loadFromFile(this.#configPath));
|
||||
const { config } = KeybindingsManager.#loadFromFile(this.#configPath);
|
||||
this.setUserBindings(config);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -437,8 +485,11 @@ export class KeybindingsManager extends TuiKeybindingsManager {
|
||||
/**
|
||||
* Load user bindings from a file, migrating old names if needed.
|
||||
*/
|
||||
static #loadFromFile(filePath: string): KeybindingsConfig {
|
||||
return loadKeybindingsConfig(filePath, true);
|
||||
static #loadFromFile(
|
||||
filePath: string,
|
||||
writeBackPath?: string,
|
||||
): { config: KeybindingsConfig; persistedPath: string } {
|
||||
return loadKeybindingsConfig(filePath, writeBackPath);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ import * as fs from "node:fs/promises";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import { setKeybindings } from "@oh-my-pi/pi-tui";
|
||||
import { YAML } from "bun";
|
||||
import { KeybindingsManager } from "../src/config/keybindings";
|
||||
|
||||
describe("KeybindingsManager.create", () => {
|
||||
@@ -14,12 +15,13 @@ describe("KeybindingsManager.create", () => {
|
||||
setKeybindings(KeybindingsManager.inMemory());
|
||||
});
|
||||
|
||||
it("migrates legacy keybinding names on disk during create", async () => {
|
||||
it("migrates legacy keybinding JSON to YAML during create", async () => {
|
||||
const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "pi-keybindings-"));
|
||||
const configPath = path.join(agentDir, "keybindings.json");
|
||||
const jsonPath = path.join(agentDir, "keybindings.json");
|
||||
const ymlPath = path.join(agentDir, "keybindings.yml");
|
||||
|
||||
await Bun.write(
|
||||
configPath,
|
||||
jsonPath,
|
||||
`${JSON.stringify(
|
||||
{
|
||||
fork: "ctrl+f",
|
||||
@@ -34,7 +36,7 @@ describe("KeybindingsManager.create", () => {
|
||||
|
||||
try {
|
||||
const manager = KeybindingsManager.create(agentDir);
|
||||
const writtenConfig = await Bun.file(configPath).json();
|
||||
const writtenConfig = YAML.parse(await Bun.file(ymlPath).text());
|
||||
|
||||
expect(manager.getKeys("app.session.fork")).toEqual(["ctrl+f"]);
|
||||
expect(manager.getKeys("tui.select.confirm")).toEqual(["enter"]);
|
||||
@@ -47,6 +49,59 @@ describe("KeybindingsManager.create", () => {
|
||||
"tui.select.confirm": "enter",
|
||||
});
|
||||
expect(writtenConfig).not.toHaveProperty("selectModelTemporary");
|
||||
expect(await Bun.file(jsonPath).exists()).toBe(true);
|
||||
} finally {
|
||||
await fs.rm(agentDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("loads keybindings.yml directly", async () => {
|
||||
const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "pi-keybindings-"));
|
||||
const configPath = path.join(agentDir, "keybindings.yml");
|
||||
|
||||
await Bun.write(
|
||||
configPath,
|
||||
YAML.stringify(
|
||||
{
|
||||
"app.session.fork": "ctrl+f",
|
||||
"app.clipboard.copyPrompt": ["alt+c", "ctrl+shift+c"],
|
||||
},
|
||||
null,
|
||||
2,
|
||||
),
|
||||
);
|
||||
|
||||
try {
|
||||
const manager = KeybindingsManager.create(agentDir);
|
||||
|
||||
expect(manager.getKeys("app.session.fork")).toEqual(["ctrl+f"]);
|
||||
expect(manager.getKeys("app.clipboard.copyPrompt")).toEqual(["alt+c", "ctrl+shift+c"]);
|
||||
} finally {
|
||||
await fs.rm(agentDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("accepts keybindings.yaml when present", async () => {
|
||||
const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "pi-keybindings-"));
|
||||
const yamlPath = path.join(agentDir, "keybindings.yaml");
|
||||
const canonicalPath = path.join(agentDir, "keybindings.yml");
|
||||
|
||||
await Bun.write(
|
||||
yamlPath,
|
||||
YAML.stringify(
|
||||
{
|
||||
"app.plan.toggle": "alt+shift+p",
|
||||
},
|
||||
null,
|
||||
2,
|
||||
),
|
||||
);
|
||||
|
||||
try {
|
||||
const manager = KeybindingsManager.create(agentDir);
|
||||
|
||||
expect(manager.getKeys("app.plan.toggle")).toEqual(["alt+shift+p"]);
|
||||
expect(await Bun.file(canonicalPath).exists()).toBe(false);
|
||||
} finally {
|
||||
await fs.rm(agentDir, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user