From 80a79683ac38326a4de2e4cc444446536f0da871 Mon Sep 17 00:00:00 2001 From: nrs <38722970+nrslib@users.noreply.github.com> Date: Thu, 19 Feb 2026 17:14:07 +0900 Subject: [PATCH] github-issue-304-builtin (#309) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * takt: github-issue-304-builtin * ピース選択UIから「also in」表示を削除 --- src/__tests__/piece-category-config.test.ts | 73 +++++++++++++++++++-- src/__tests__/piece-selection.test.ts | 61 ++++++++++++++++- src/features/pieceSelection/index.ts | 19 +----- src/infra/config/loaders/index.ts | 1 + src/infra/config/loaders/pieceCategories.ts | 64 +++++++----------- 5 files changed, 153 insertions(+), 65 deletions(-) diff --git a/src/__tests__/piece-category-config.test.ts b/src/__tests__/piece-category-config.test.ts index 475fc44..123f10f 100644 --- a/src/__tests__/piece-category-config.test.ts +++ b/src/__tests__/piece-category-config.test.ts @@ -61,6 +61,7 @@ vi.mock('../infra/config/global/pieceCategories.js', async (importOriginal) => { }); const { + BUILTIN_CATEGORY_NAME, getPieceCategories, loadDefaultCategories, buildCategorizedPieces, @@ -129,6 +130,7 @@ piece_categories: { name: 'Quick Start', pieces: ['default'], children: [] }, ]); expect(config!.userPieceCategories).toEqual([]); + expect(config!.hasUserCategories).toBe(false); }); it('should use builtin categories when user overlay file is missing', () => { @@ -147,11 +149,12 @@ others_category_name: Others { name: 'Main', pieces: ['default'], children: [] }, ]); expect(config!.userPieceCategories).toEqual([]); + expect(config!.hasUserCategories).toBe(false); expect(config!.showOthersCategory).toBe(true); expect(config!.othersCategoryName).toBe('Others'); }); - it('should merge user overlay categories with builtin categories', () => { + it('should separate user categories from builtin categories with builtin wrapper', () => { writeYaml(join(resourcesDir, 'piece-categories.yaml'), ` piece_categories: Main: @@ -184,15 +187,22 @@ others_category_name: Unclassified const config = getPieceCategories(testDir); expect(config).not.toBeNull(); expect(config!.pieceCategories).toEqual([ + { name: 'Main', pieces: ['custom'], children: [] }, + { name: 'My Team', pieces: ['team-flow'], children: [] }, { - name: 'Main', - pieces: ['custom'], + name: BUILTIN_CATEGORY_NAME, + pieces: [], children: [ - { name: 'Child', pieces: ['nested'], children: [] }, + { + name: 'Main', + pieces: ['default', 'coding'], + children: [ + { name: 'Child', pieces: ['nested'], children: [] }, + ], + }, + { name: 'Review', pieces: ['review-only', 'e2e-test'], children: [] }, ], }, - { name: 'Review', pieces: ['review-only', 'e2e-test'], children: [] }, - { name: 'My Team', pieces: ['team-flow'], children: [] }, ]); expect(config!.builtinPieceCategories).toEqual([ { @@ -208,6 +218,7 @@ others_category_name: Unclassified { name: 'Main', pieces: ['custom'], children: [] }, { name: 'My Team', pieces: ['team-flow'], children: [] }, ]); + expect(config!.hasUserCategories).toBe(true); expect(config!.showOthersCategory).toBe(false); expect(config!.othersCategoryName).toBe('Unclassified'); }); @@ -259,6 +270,7 @@ others_category_name: Unclassified { name: 'Review', pieces: ['review-only'], children: [] }, ]); expect(config!.userPieceCategories).toEqual([]); + expect(config!.hasUserCategories).toBe(false); expect(config!.showOthersCategory).toBe(false); expect(config!.othersCategoryName).toBe('Unclassified'); }); @@ -290,6 +302,7 @@ describe('buildCategorizedPieces', () => { userPieceCategories: [ { name: 'My Team', pieces: ['missing-user-piece'], children: [] }, ], + hasUserCategories: true, showOthersCategory: true, othersCategoryName: 'Others', }; @@ -322,6 +335,7 @@ describe('buildCategorizedPieces', () => { { name: 'Main', pieces: ['default'], children: [] }, ], userPieceCategories: [], + hasUserCategories: false, showOthersCategory: true, othersCategoryName: 'Others', }; @@ -346,6 +360,7 @@ describe('buildCategorizedPieces', () => { { name: 'Main', pieces: ['default'], children: [] }, ], userPieceCategories: [], + hasUserCategories: false, showOthersCategory: false, othersCategoryName: 'Others', }; @@ -356,6 +371,52 @@ describe('buildCategorizedPieces', () => { ]); }); + it('should categorize pieces through builtin wrapper node', () => { + const allPieces = createPieceMap([ + { name: 'custom', source: 'user' }, + { name: 'default', source: 'builtin' }, + { name: 'review-only', source: 'builtin' }, + { name: 'extra', source: 'builtin' }, + ]); + const config = { + pieceCategories: [ + { name: 'My Team', pieces: ['custom'], children: [] }, + { + name: BUILTIN_CATEGORY_NAME, + pieces: [], + children: [ + { name: 'Quick Start', pieces: ['default'], children: [] }, + { name: 'Review', pieces: ['review-only'], children: [] }, + ], + }, + ], + builtinPieceCategories: [ + { name: 'Quick Start', pieces: ['default'], children: [] }, + { name: 'Review', pieces: ['review-only'], children: [] }, + ], + userPieceCategories: [ + { name: 'My Team', pieces: ['custom'], children: [] }, + ], + hasUserCategories: true, + showOthersCategory: true, + othersCategoryName: 'Others', + }; + + const categorized = buildCategorizedPieces(allPieces, config); + expect(categorized.categories).toEqual([ + { name: 'My Team', pieces: ['custom'], children: [] }, + { + name: BUILTIN_CATEGORY_NAME, + pieces: [], + children: [ + { name: 'Quick Start', pieces: ['default'], children: [] }, + { name: 'Review', pieces: ['review-only'], children: [] }, + ], + }, + { name: 'Others', pieces: ['extra'], children: [] }, + ]); + }); + it('should find categories containing a piece', () => { const categories = [ { name: 'A', pieces: ['shared'], children: [] }, diff --git a/src/__tests__/piece-selection.test.ts b/src/__tests__/piece-selection.test.ts index feab25c..688bd61 100644 --- a/src/__tests__/piece-selection.test.ts +++ b/src/__tests__/piece-selection.test.ts @@ -39,8 +39,8 @@ const configMock = vi.hoisted(() => ({ loadAllPiecesWithSources: vi.fn(), getPieceCategories: vi.fn(), buildCategorizedPieces: vi.fn(), + getCurrentPiece: vi.fn(), resolveConfigValue: vi.fn(), - findPieceCategories: vi.fn(() => []), })); vi.mock('../infra/config/index.js', () => configMock); @@ -242,6 +242,65 @@ describe('selectPieceFromCategorizedPieces', () => { // Should NOT contain the parent category again expect(labels.some((l) => l.includes('Dev'))).toBe(false); }); + + it('should navigate into builtin wrapper category and select a piece', async () => { + const categorized: CategorizedPieces = { + categories: [ + { name: 'My Team', pieces: ['custom'], children: [] }, + { + name: 'builtin', + pieces: [], + children: [ + { name: 'Quick Start', pieces: ['default'], children: [] }, + ], + }, + ], + allPieces: createPieceMap([ + { name: 'custom', source: 'user' }, + { name: 'default', source: 'builtin' }, + ]), + missingPieces: [], + }; + + // Select builtin category → Quick Start subcategory → piece + selectOptionMock + .mockResolvedValueOnce('__custom_category__:builtin') + .mockResolvedValueOnce('__category__:Quick Start') + .mockResolvedValueOnce('default'); + + const selected = await selectPieceFromCategorizedPieces(categorized, ''); + expect(selected).toBe('default'); + expect(selectOptionMock).toHaveBeenCalledTimes(3); + }); + + it('should show builtin wrapper as a folder in top-level options', async () => { + const categorized: CategorizedPieces = { + categories: [ + { name: 'My Team', pieces: ['custom'], children: [] }, + { + name: 'builtin', + pieces: [], + children: [ + { name: 'Quick Start', pieces: ['default'], children: [] }, + ], + }, + ], + allPieces: createPieceMap([ + { name: 'custom', source: 'user' }, + { name: 'default', source: 'builtin' }, + ]), + missingPieces: [], + }; + + selectOptionMock.mockResolvedValueOnce(null); + + await selectPieceFromCategorizedPieces(categorized, ''); + + const firstCallOptions = selectOptionMock.mock.calls[0]![1] as { label: string; value: string }[]; + const labels = firstCallOptions.map((o) => o.label); + expect(labels.some((l) => l.includes('My Team'))).toBe(true); + expect(labels.some((l) => l.includes('builtin'))).toBe(true); + }); }); describe('selectPiece', () => { diff --git a/src/features/pieceSelection/index.ts b/src/features/pieceSelection/index.ts index 7aefa74..2f6fdfd 100644 --- a/src/features/pieceSelection/index.ts +++ b/src/features/pieceSelection/index.ts @@ -11,7 +11,6 @@ import { removeBookmark, } from '../../infra/config/global/index.js'; import { - findPieceCategories, listPieces, listPieceEntries, loadAllPiecesWithSources, @@ -160,8 +159,6 @@ function buildCategoryLevelOptions( categories: PieceCategoryNode[], pieces: string[], currentPiece: string, - rootCategories: PieceCategoryNode[], - currentPathLabel: string, ): { options: SelectionOption[]; categoryMap: Map; @@ -181,19 +178,7 @@ function buildCategoryLevelOptions( for (const pieceName of pieces) { const isCurrent = pieceName === currentPiece; - const alsoIn = findPieceCategories(pieceName, rootCategories) - .filter((path) => path !== currentPathLabel); - const alsoInLabel = alsoIn.length > 0 ? `also in ${alsoIn.join(', ')}` : ''; - - let label = `🎼 ${pieceName}`; - if (isCurrent && alsoInLabel) { - label = `🎼 ${pieceName} (current, ${alsoInLabel})`; - } else if (isCurrent) { - label = `🎼 ${pieceName} (current)`; - } else if (alsoInLabel) { - label = `🎼 ${pieceName} (${alsoInLabel})`; - } - + const label = isCurrent ? `🎼 ${pieceName} (current)` : `🎼 ${pieceName}`; options.push({ label, value: pieceName }); } @@ -223,8 +208,6 @@ async function selectPieceFromCategoryTree( currentCategories, currentPieces, currentPiece, - categories, - currentPathLabel, ); if (options.length === 0) { diff --git a/src/infra/config/loaders/index.ts b/src/infra/config/loaders/index.ts index dca855a..81a8af2 100644 --- a/src/infra/config/loaders/index.ts +++ b/src/infra/config/loaders/index.ts @@ -20,6 +20,7 @@ export { } from './pieceLoader.js'; export { + BUILTIN_CATEGORY_NAME, loadDefaultCategories, getDefaultCategoriesPath, getPieceCategories, diff --git a/src/infra/config/loaders/pieceCategories.ts b/src/infra/config/loaders/pieceCategories.ts index a503e46..70410cd 100644 --- a/src/infra/config/loaders/pieceCategories.ts +++ b/src/infra/config/loaders/pieceCategories.ts @@ -22,6 +22,8 @@ const CategoryConfigSchema = z.object({ others_category_name: z.string().min(1).optional(), }).passthrough(); +export const BUILTIN_CATEGORY_NAME = 'builtin'; + export interface PieceCategoryNode { name: string; pieces: string[]; @@ -32,6 +34,7 @@ export interface CategoryConfig { pieceCategories: PieceCategoryNode[]; builtinPieceCategories: PieceCategoryNode[]; userPieceCategories: PieceCategoryNode[]; + hasUserCategories: boolean; showOthersCategory: boolean; othersCategoryName: string; } @@ -57,7 +60,6 @@ interface RawCategoryConfig { interface ParsedCategoryNode { name: string; pieces: string[]; - hasPieces: boolean; children: ParsedCategoryNode[]; } @@ -97,7 +99,6 @@ function parseCategoryNode( throw new Error(`category "${name}" must be an object in ${sourceLabel} at ${path.join(' > ')}`); } - const hasPieces = Object.prototype.hasOwnProperty.call(raw, 'pieces'); const pieces = parsePieces(raw.pieces, sourceLabel, path); const children: ParsedCategoryNode[] = []; @@ -109,7 +110,7 @@ function parseCategoryNode( children.push(parseCategoryNode(key, value, sourceLabel, [...path, key])); } - return { name, pieces, hasPieces, children }; + return { name, pieces, children }; } function parseCategoryTree(raw: unknown, sourceLabel: string): ParsedCategoryNode[] { @@ -176,38 +177,6 @@ function convertParsedNodes(nodes: ParsedCategoryNode[]): PieceCategoryNode[] { })); } -function mergeCategoryNodes(baseNodes: ParsedCategoryNode[], overlayNodes: ParsedCategoryNode[]): ParsedCategoryNode[] { - const overlayByName = new Map(); - for (const overlayNode of overlayNodes) { - overlayByName.set(overlayNode.name, overlayNode); - } - - const merged: ParsedCategoryNode[] = []; - for (const baseNode of baseNodes) { - const overlayNode = overlayByName.get(baseNode.name); - if (!overlayNode) { - merged.push(baseNode); - continue; - } - - overlayByName.delete(baseNode.name); - - const mergedNode: ParsedCategoryNode = { - name: baseNode.name, - pieces: overlayNode.hasPieces ? overlayNode.pieces : baseNode.pieces, - hasPieces: baseNode.hasPieces || overlayNode.hasPieces, - children: mergeCategoryNodes(baseNode.children, overlayNode.children), - }; - merged.push(mergedNode); - } - - for (const overlayNode of overlayByName.values()) { - merged.push(overlayNode); - } - - return merged; -} - function resolveShowOthersCategory(defaultConfig: ParsedCategoryConfig, userConfig: ParsedCategoryConfig | null): boolean { if (userConfig?.showOthersCategory !== undefined) { return userConfig.showOthersCategory; @@ -249,6 +218,7 @@ export function loadDefaultCategories(cwd: string): CategoryConfig | null { pieceCategories: builtinPieceCategories, builtinPieceCategories, userPieceCategories: [], + hasUserCategories: false, showOthersCategory, othersCategoryName, }; @@ -260,6 +230,18 @@ export function getDefaultCategoriesPath(cwd: string): string { return join(getLanguageResourcesDir(lang), 'piece-categories.yaml'); } +function buildSeparatedCategories( + userCategories: PieceCategoryNode[], + builtinCategories: PieceCategoryNode[], +): PieceCategoryNode[] { + const builtinWrapper: PieceCategoryNode = { + name: BUILTIN_CATEGORY_NAME, + pieces: [], + children: builtinCategories, + }; + return [...userCategories, builtinWrapper]; +} + /** * Get effective piece categories configuration. * Built from builtin categories and optional user overlay. @@ -274,17 +256,19 @@ export function getPieceCategories(cwd: string): CategoryConfig | null { const userPath = getPieceCategoriesPath(cwd); const userConfig = loadCategoryConfigFromPath(userPath, userPath); - const merged = userConfig?.pieceCategories - ? mergeCategoryNodes(defaultConfig.pieceCategories, userConfig.pieceCategories) - : defaultConfig.pieceCategories; - const builtinPieceCategories = convertParsedNodes(defaultConfig.pieceCategories); const userPieceCategories = convertParsedNodes(userConfig?.pieceCategories ?? []); + const hasUserCategories = userPieceCategories.length > 0; + + const pieceCategories = hasUserCategories + ? buildSeparatedCategories(userPieceCategories, builtinPieceCategories) + : builtinPieceCategories; return { - pieceCategories: convertParsedNodes(merged), + pieceCategories, builtinPieceCategories, userPieceCategories, + hasUserCategories, showOthersCategory: resolveShowOthersCategory(defaultConfig, userConfig), othersCategoryName: resolveOthersCategoryName(defaultConfig, userConfig), };