Skip to content

fix: avoid creating redundant snippet/ dir when snippets/ already exists + path tracking refactor #34

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bew wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: avoid creating redundant snippet/ dir when snippets/ already exists + path tracking refactor #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/commands.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { parseCommandArgs } from "./arg-parser.js";
import { getProjectPaths, PATHS } from "./constants.js";
import { GLOBAL_PATHS, getProjectPaths } from "./constants.js";
import { createSnippet, deleteSnippet, listSnippets, reloadSnippets } from "./loader.js";
import { logger } from "./logger.js";
import { sendIgnoredMessage } from "./notification.js";
Expand Down Expand Up @@ -352,12 +352,12 @@ function formatAliases(aliases: string[]): string {
}

function globalSnippetLocations(): string {
return `${PATHS.SNIPPETS_DIR}/ or ${PATHS.SNIPPETS_DIR_ALT}/`;
return `${GLOBAL_PATHS.SNIPPETS_DIR_PREFERRED}/ or ${GLOBAL_PATHS.SNIPPETS_DIR_ALT}/`;
}

function projectSnippetLocations(projectDir: string): string {
const paths = getProjectPaths(projectDir);
return `${paths.SNIPPETS_DIR}/ or ${paths.SNIPPETS_DIR_ALT}/`;
return `${paths.SNIPPETS_DIR_PREFERRED}/ or ${paths.SNIPPETS_DIR_ALT}/`;
}

/**
Expand Down
82 changes: 39 additions & 43 deletions src/config.integration.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { mkdirSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { loadConfig } from "./config.js";
import { GLOBAL_PATHS } from "./constants.js";
import { logger } from "./logger.js";

describe("Config Integration", () => {
Expand All @@ -27,22 +28,35 @@ describe("Config Integration", () => {
logger.debugEnabled = false;
});

function withGlobalDir(fn: () => void) {
const origPreferred = GLOBAL_PATHS.SNIPPETS_DIR_PREFERRED;
const origActive = GLOBAL_PATHS.ACTIVE_SNIPPETS_DIR;
const origAlt = GLOBAL_PATHS.SNIPPETS_DIR_ALT;
const origConfig = GLOBAL_PATHS.CONFIG_FILE;

GLOBAL_PATHS.SNIPPETS_DIR_PREFERRED = globalDir;
GLOBAL_PATHS.ACTIVE_SNIPPETS_DIR = globalDir;
GLOBAL_PATHS.SNIPPETS_DIR_ALT = join(globalDir, ".nonexistent-alt");
GLOBAL_PATHS.CONFIG_FILE = join(globalDir, "config.jsonc");

try {
fn();
} finally {
GLOBAL_PATHS.SNIPPETS_DIR_PREFERRED = origPreferred;
GLOBAL_PATHS.ACTIVE_SNIPPETS_DIR = origActive;
GLOBAL_PATHS.SNIPPETS_DIR_ALT = origAlt;
GLOBAL_PATHS.CONFIG_FILE = origConfig;
}
}

describe("logging.debug config", () => {
it("should enable debug logging when config.logging.debug is true", () => {
writeFileSync(join(globalDir, "config.jsonc"), JSON.stringify({ logging: { debug: true } }));

// Temporarily override PATHS for this test
const originalPaths = require("./constants.js").PATHS;
require("./constants.js").PATHS.CONFIG_FILE_GLOBAL = join(globalDir, "config.jsonc");
require("./constants.js").PATHS.SNIPPETS_DIR = globalDir;

const config = loadConfig();

expect(config.logging.debug).toBe(true);

// Restore
require("./constants.js").PATHS.CONFIG_FILE_GLOBAL = originalPaths.CONFIG_FILE_GLOBAL;
require("./constants.js").PATHS.SNIPPETS_DIR = originalPaths.SNIPPETS_DIR;
withGlobalDir(() => {
const config = loadConfig();
expect(config.logging.debug).toBe(true);
});
});

it("should accept 'enabled' string for debug logging", () => {
Expand All @@ -51,16 +65,10 @@ describe("Config Integration", () => {
JSON.stringify({ logging: { debug: "enabled" } }),
);

const originalPaths = require("./constants.js").PATHS;
require("./constants.js").PATHS.CONFIG_FILE_GLOBAL = join(globalDir, "config.jsonc");
require("./constants.js").PATHS.SNIPPETS_DIR = globalDir;

const config = loadConfig();

expect(config.logging.debug).toBe(true);

require("./constants.js").PATHS.CONFIG_FILE_GLOBAL = originalPaths.CONFIG_FILE_GLOBAL;
require("./constants.js").PATHS.SNIPPETS_DIR = originalPaths.SNIPPETS_DIR;
withGlobalDir(() => {
const config = loadConfig();
expect(config.logging.debug).toBe(true);
});
});
});

Expand All @@ -75,16 +83,10 @@ describe("Config Integration", () => {
JSON.stringify({ logging: { debug: true } }),
);

const originalPaths = require("./constants.js").PATHS;
require("./constants.js").PATHS.CONFIG_FILE_GLOBAL = join(globalDir, "config.jsonc");
require("./constants.js").PATHS.SNIPPETS_DIR = globalDir;

const config = loadConfig(projectDir);

expect(config.logging.debug).toBe(true);

require("./constants.js").PATHS.CONFIG_FILE_GLOBAL = originalPaths.CONFIG_FILE_GLOBAL;
require("./constants.js").PATHS.SNIPPETS_DIR = originalPaths.SNIPPETS_DIR;
withGlobalDir(() => {
const config = loadConfig(projectDir);
expect(config.logging.debug).toBe(true);
});
});

it("should merge partial project config", () => {
Expand All @@ -100,17 +102,11 @@ describe("Config Integration", () => {
JSON.stringify({ logging: { debug: true } }),
);

const originalPaths = require("./constants.js").PATHS;
require("./constants.js").PATHS.CONFIG_FILE_GLOBAL = join(globalDir, "config.jsonc");
require("./constants.js").PATHS.SNIPPETS_DIR = globalDir;

const config = loadConfig(projectDir);

expect(config.logging.debug).toBe(true);
expect(config.injectRecencyMessages).toBe(9); // inherited from global

require("./constants.js").PATHS.CONFIG_FILE_GLOBAL = originalPaths.CONFIG_FILE_GLOBAL;
require("./constants.js").PATHS.SNIPPETS_DIR = originalPaths.SNIPPETS_DIR;
withGlobalDir(() => {
const config = loadConfig(projectDir);
expect(config.logging.debug).toBe(true);
expect(config.injectRecencyMessages).toBe(9); // inherited from global
});
});
});
});
79 changes: 38 additions & 41 deletions src/config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,20 @@ import { afterEach, beforeEach, describe, expect, it } from "bun:test";
import { existsSync, mkdirSync, rmSync, writeFileSync } from "node:fs";
import { join } from "node:path";
import { getGlobalConfigPath, getProjectConfigPath, loadConfig } from "./config.js";
import { PATHS } from "./constants.js";
import { GLOBAL_PATHS } from "./constants.js";

// Use temp directories for testing to avoid affecting real config
const TEST_TEMP_DIR = join(import.meta.dirname ?? ".", ".test-temp-config");
const TEST_GLOBAL_SNIPPETS_DIR = join(TEST_TEMP_DIR, "global", "snippet");
const TEST_GLOBAL_SNIPPETS_DIR_ALT = join(TEST_TEMP_DIR, "global", "snippets"); // nonexistent
const TEST_PROJECT_DIR = join(TEST_TEMP_DIR, "project");
const TEST_PROJECT_SNIPPETS_DIR = join(TEST_PROJECT_DIR, ".opencode", "snippet");
const TEST_GLOBAL_CONFIG_FILE = join(TEST_GLOBAL_SNIPPETS_DIR, "config.jsonc");

// Store original PATHS values to restore after tests
const originalSnippetsDir = PATHS.SNIPPETS_DIR;
const originalConfigFileGlobal = (PATHS as Record<string, string>).CONFIG_FILE_GLOBAL;
// Store original GLOBAL_PATHS values to restore after tests
const originalActiveSnippetsDir = GLOBAL_PATHS.ACTIVE_SNIPPETS_DIR;
const originalSnippetsDirAlt = GLOBAL_PATHS.SNIPPETS_DIR_ALT;
const originalConfigFile = GLOBAL_PATHS.CONFIG_FILE;

describe("config", () => {
beforeEach(() => {
Expand All @@ -25,28 +28,38 @@ describe("config", () => {
mkdirSync(TEST_GLOBAL_SNIPPETS_DIR, { recursive: true });
mkdirSync(TEST_PROJECT_SNIPPETS_DIR, { recursive: true });

// Override PATHS for testing (using Object.defineProperty since PATHS is readonly)
Object.defineProperty(PATHS, "SNIPPETS_DIR", {
// Override GLOBAL_PATHS for testing (using Object.defineProperty since GLOBAL_PATHS is readonly)
Object.defineProperty(GLOBAL_PATHS, "ACTIVE_SNIPPETS_DIR", {
value: TEST_GLOBAL_SNIPPETS_DIR,
writable: true,
configurable: true,
});
Object.defineProperty(PATHS, "CONFIG_FILE_GLOBAL", {
value: join(TEST_GLOBAL_SNIPPETS_DIR, "config.jsonc"),
Object.defineProperty(GLOBAL_PATHS, "SNIPPETS_DIR_ALT", {
value: TEST_GLOBAL_SNIPPETS_DIR_ALT, // nonexistent, so alt is never chosen
writable: true,
configurable: true,
});
Object.defineProperty(GLOBAL_PATHS, "CONFIG_FILE", {
value: TEST_GLOBAL_CONFIG_FILE,
writable: true,
configurable: true,
});
});

afterEach(() => {
// Restore original PATHS
Object.defineProperty(PATHS, "SNIPPETS_DIR", {
value: originalSnippetsDir,
// Restore original GLOBAL_PATHS
Object.defineProperty(GLOBAL_PATHS, "ACTIVE_SNIPPETS_DIR", {
value: originalActiveSnippetsDir,
writable: true,
configurable: true,
});
Object.defineProperty(PATHS, "CONFIG_FILE_GLOBAL", {
value: originalConfigFileGlobal,
Object.defineProperty(GLOBAL_PATHS, "SNIPPETS_DIR_ALT", {
value: originalSnippetsDirAlt,
writable: true,
configurable: true,
});
Object.defineProperty(GLOBAL_PATHS, "CONFIG_FILE", {
value: originalConfigFile,
writable: true,
configurable: true,
});
Expand All @@ -70,20 +83,16 @@ describe("config", () => {

it("should auto-create global config file when it doesn't exist", () => {
// Config file should not exist initially
expect(existsSync((PATHS as Record<string, string>).CONFIG_FILE_GLOBAL)).toBe(false);
expect(existsSync(TEST_GLOBAL_CONFIG_FILE)).toBe(false);

loadConfig();

// Config file should be created
expect(existsSync((PATHS as Record<string, string>).CONFIG_FILE_GLOBAL)).toBe(true);
expect(existsSync(TEST_GLOBAL_CONFIG_FILE)).toBe(true);
});

it("should load global config file", () => {
writeFileSync(
(PATHS as Record<string, string>).CONFIG_FILE_GLOBAL,
JSON.stringify({ logging: { debug: true } }),
"utf-8",
);
writeFileSync(TEST_GLOBAL_CONFIG_FILE, JSON.stringify({ logging: { debug: true } }), "utf-8");

const config = loadConfig();

Expand All @@ -98,7 +107,7 @@ describe("config", () => {
"debug": true
}
}`;
writeFileSync((PATHS as Record<string, string>).CONFIG_FILE_GLOBAL, jsoncContent, "utf-8");
writeFileSync(TEST_GLOBAL_CONFIG_FILE, jsoncContent, "utf-8");

const config = loadConfig();

Expand All @@ -107,7 +116,7 @@ describe("config", () => {

it("should accept 'enabled' string for debug", () => {
writeFileSync(
(PATHS as Record<string, string>).CONFIG_FILE_GLOBAL,
TEST_GLOBAL_CONFIG_FILE,
JSON.stringify({ logging: { debug: "enabled" } }),
"utf-8",
);
Expand All @@ -119,7 +128,7 @@ describe("config", () => {

it("should accept 'disabled' string for debug", () => {
writeFileSync(
(PATHS as Record<string, string>).CONFIG_FILE_GLOBAL,
TEST_GLOBAL_CONFIG_FILE,
JSON.stringify({ logging: { debug: "disabled" } }),
"utf-8",
);
Expand All @@ -131,11 +140,7 @@ describe("config", () => {

it("should merge partial config with defaults", () => {
// Only set debug, other options should use default
writeFileSync(
(PATHS as Record<string, string>).CONFIG_FILE_GLOBAL,
JSON.stringify({ logging: { debug: true } }),
"utf-8",
);
writeFileSync(TEST_GLOBAL_CONFIG_FILE, JSON.stringify({ logging: { debug: true } }), "utf-8");

const config = loadConfig();

Expand All @@ -145,11 +150,7 @@ describe("config", () => {

it("should merge project config with global config (project has priority)", () => {
// Global config
writeFileSync(
(PATHS as Record<string, string>).CONFIG_FILE_GLOBAL,
JSON.stringify({ logging: { debug: true } }),
"utf-8",
);
writeFileSync(TEST_GLOBAL_CONFIG_FILE, JSON.stringify({ logging: { debug: true } }), "utf-8");

// Project config (overrides global)
const projectConfigPath = join(TEST_PROJECT_SNIPPETS_DIR, "config.jsonc");
Expand All @@ -161,11 +162,7 @@ describe("config", () => {
});

it("should handle malformed JSONC gracefully", () => {
writeFileSync(
(PATHS as Record<string, string>).CONFIG_FILE_GLOBAL,
"{ invalid json }",
"utf-8",
);
writeFileSync(TEST_GLOBAL_CONFIG_FILE, "{ invalid json }", "utf-8");

const config = loadConfig();

Expand All @@ -179,7 +176,7 @@ describe("config", () => {

it("should load experimental skill loading config", () => {
writeFileSync(
(PATHS as Record<string, string>).CONFIG_FILE_GLOBAL,
TEST_GLOBAL_CONFIG_FILE,
JSON.stringify({ experimental: { skillLoading: true } }),
"utf-8",
);
Expand All @@ -191,7 +188,7 @@ describe("config", () => {

it("should ignore invalid config value types", () => {
writeFileSync(
(PATHS as Record<string, string>).CONFIG_FILE_GLOBAL,
TEST_GLOBAL_CONFIG_FILE,
JSON.stringify({ logging: { debug: "yes" } }),
"utf-8",
);
Expand All @@ -206,7 +203,7 @@ describe("config", () => {
describe("getGlobalConfigPath", () => {
it("should return the global config path", () => {
const path = getGlobalConfigPath();
expect(path).toBe((PATHS as Record<string, string>).CONFIG_FILE_GLOBAL);
expect(path).toBe(TEST_GLOBAL_CONFIG_FILE);
});
});

Expand Down
22 changes: 11 additions & 11 deletions src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
import { importCjs } from "./cjs-interop.js";
import { getProjectPaths, PATHS } from "./constants.js";
import { GLOBAL_PATHS, getProjectPaths } from "./constants.js";
import { logger } from "./logger.js";

const { parse: parseJsonc } = await importCjs<typeof import("jsonc-parser")>("jsonc-parser");
Expand Down Expand Up @@ -159,15 +159,15 @@ function parseJsoncFile(filePath: string): RawConfig {
*/
function ensureGlobalConfigExists(): void {
// Create snippets directory if it doesn't exist
if (!existsSync(PATHS.SNIPPETS_DIR)) {
mkdirSync(PATHS.SNIPPETS_DIR, { recursive: true });
logger.debug("Created global snippets directory", { path: PATHS.SNIPPETS_DIR });
if (!existsSync(GLOBAL_PATHS.ACTIVE_SNIPPETS_DIR)) {
mkdirSync(GLOBAL_PATHS.ACTIVE_SNIPPETS_DIR, { recursive: true });
logger.debug("Created global snippets directory", { path: GLOBAL_PATHS.ACTIVE_SNIPPETS_DIR });
}

// Create default config file if it doesn't exist
if (!existsSync(PATHS.CONFIG_FILE_GLOBAL)) {
writeFileSync(PATHS.CONFIG_FILE_GLOBAL, DEFAULT_CONFIG_CONTENT, "utf-8");
logger.debug("Created default config file", { path: PATHS.CONFIG_FILE_GLOBAL });
if (!existsSync(GLOBAL_PATHS.CONFIG_FILE)) {
writeFileSync(GLOBAL_PATHS.CONFIG_FILE, DEFAULT_CONFIG_CONTENT, "utf-8");
logger.debug("Created default config file", { path: GLOBAL_PATHS.CONFIG_FILE });
}
}

Expand All @@ -190,10 +190,10 @@ export function loadConfig(projectDir?: string): SnippetsConfig {
let config: SnippetsConfig = structuredClone(DEFAULT_CONFIG);

// Load global config
if (existsSync(PATHS.CONFIG_FILE_GLOBAL)) {
const globalConfig = parseJsoncFile(PATHS.CONFIG_FILE_GLOBAL);
if (existsSync(GLOBAL_PATHS.CONFIG_FILE)) {
const globalConfig = parseJsoncFile(GLOBAL_PATHS.CONFIG_FILE);
config = mergeConfig(config, globalConfig);
logger.debug("Loaded global config", { path: PATHS.CONFIG_FILE_GLOBAL });
logger.debug("Loaded global config", { path: GLOBAL_PATHS.CONFIG_FILE });
}

// Load project config if project directory is provided
Expand Down Expand Up @@ -247,7 +247,7 @@ function mergeConfig(base: SnippetsConfig, raw: RawConfig): SnippetsConfig {
* Get the path to the global config file
*/
export function getGlobalConfigPath(): string {
return PATHS.CONFIG_FILE_GLOBAL;
return GLOBAL_PATHS.CONFIG_FILE;
}

/**
Expand Down
Loading