Merge pull request #189 from KubrickCode/develop/shlee/188

feat(button-set): add rename button set feature

Author: KubrickCode

src/internal/managers/button-set-manager.spec.ts

@@ -641,7 +641,126 @@ describe("ButtonSetManager", () => {
     });
   });
 
-  describe("updateButtonSet", () => {
+  describe("renameButtonSet", () => {
+    it("should return setNotFound error when renaming non-existent set", async () => {
+      const existingSets = [{ buttons: [], id: "set-1", name: "SetA" }];
+      const mockConfig = createMockConfig();
+      mockConfig.get.mockReturnValue(CONFIGURATION_TARGETS.WORKSPACE);
+      mockConfig.inspect.mockImplementation((key: string) => {
+        if (key === "buttonSets") {
+          return { workspaceValue: existingSets };
+        }
+        if (key === "activeSet") {
+          return { workspaceValue: null };
+        }
+        return {};
+      });
+      jest
+        .spyOn(vscode.workspace, "getConfiguration")
+        .mockReturnValue(mockConfig as unknown as vscode.WorkspaceConfiguration);
+
+      const configManager = createMockConfigManager();
+      const configReader = createMockConfigReader();
+      const buttonSetWriter = createMockButtonSetWriter();
+
+      const manager = ButtonSetManager.create(configManager, configReader, buttonSetWriter);
+      const result = await manager.renameButtonSet("NonExistent", "NewName");
+
+      expect(result.success).toBe(false);
+      if (!result.success) {
+        expect(result.error).toBe("setNotFound");
+      }
+    });
+
+    it("should return setNameRequired error when new name is empty", async () => {
+      const existingSets = [{ buttons: [], id: "set-1", name: "SetA" }];
+      const mockConfig = createMockConfig();
+      mockConfig.get.mockReturnValue(CONFIGURATION_TARGETS.WORKSPACE);
+      mockConfig.inspect.mockImplementation((key: string) => {
+        if (key === "buttonSets") {
+          return { workspaceValue: existingSets };
+        }
+        if (key === "activeSet") {
+          return { workspaceValue: null };
+        }
+        return {};
+      });
+      jest
+        .spyOn(vscode.workspace, "getConfiguration")
+        .mockReturnValue(mockConfig as unknown as vscode.WorkspaceConfiguration);
+
+      const configManager = createMockConfigManager();
+      const configReader = createMockConfigReader();
+      const buttonSetWriter = createMockButtonSetWriter();
+
+      const manager = ButtonSetManager.create(configManager, configReader, buttonSetWriter);
+      const result = await manager.renameButtonSet("SetA", "   ");
+
+      expect(result.success).toBe(false);
+      if (!result.success) {
+        expect(result.error).toBe("setNameRequired");
+      }
+    });
+
+    it("should update active set when renaming currently active set", async () => {
+      const existingSets = [{ buttons: [], id: "set-1", name: "SetA" }];
+      const mockConfig = createMockConfig();
+      mockConfig.get.mockReturnValue(CONFIGURATION_TARGETS.WORKSPACE);
+      mockConfig.inspect.mockImplementation((key: string) => {
+        if (key === "buttonSets") {
+          return { workspaceValue: existingSets };
+        }
+        if (key === "activeSet") {
+          return { workspaceValue: "SetA" };
+        }
+        return {};
+      });
+      jest
+        .spyOn(vscode.workspace, "getConfiguration")
+        .mockReturnValue(mockConfig as unknown as vscode.WorkspaceConfiguration);
+
+      const configManager = createMockConfigManager();
+      const configReader = createMockConfigReader();
+      const buttonSetWriter = createMockButtonSetWriter();
+
+      const manager = ButtonSetManager.create(configManager, configReader, buttonSetWriter);
+      const result = await manager.renameButtonSet("SetA", "SetB");
+
+      expect(result.success).toBe(true);
+      expect(buttonSetWriter.writeActiveSet).toHaveBeenCalledWith("SetB", expect.anything());
+    });
+
+    it("should NOT update active set when renaming non-active set", async () => {
+      const existingSets = [
+        { buttons: [], id: "set-1", name: "SetA" },
+        { buttons: [], id: "set-2", name: "SetB" },
+      ];
+      const mockConfig = createMockConfig();
+      mockConfig.get.mockReturnValue(CONFIGURATION_TARGETS.WORKSPACE);
+      mockConfig.inspect.mockImplementation((key: string) => {
+        if (key === "buttonSets") {
+          return { workspaceValue: existingSets };
+        }
+        if (key === "activeSet") {
+          return { workspaceValue: "SetA" };
+        }
+        return {};
+      });
+      jest
+        .spyOn(vscode.workspace, "getConfiguration")
+        .mockReturnValue(mockConfig as unknown as vscode.WorkspaceConfiguration);
+
+      const configManager = createMockConfigManager();
+      const configReader = createMockConfigReader();
+      const buttonSetWriter = createMockButtonSetWriter();
+
+      const manager = ButtonSetManager.create(configManager, configReader, buttonSetWriter);
+      const result = await manager.renameButtonSet("SetB", "SetC");
+
+      expect(result.success).toBe(true);
+      expect(buttonSetWriter.writeActiveSet).not.toHaveBeenCalled();
+    });
+
     it("should detect duplicate when renaming to existing name with different case", async () => {
       const existingSets = [
         { buttons: [], id: "set-1", name: "SetA" },
@@ -669,7 +788,7 @@ describe("ButtonSetManager", () => {
       const buttonSetWriter = createMockButtonSetWriter();
 
       const manager = ButtonSetManager.create(configManager, configReader, buttonSetWriter);
-      const result = await manager.updateButtonSet("set-1", { name: "SETB" });
+      const result = await manager.renameButtonSet("SetA", "SETB");
 
       expect(result.success).toBe(false);
       if (!result.success) {
@@ -704,7 +823,7 @@ describe("ButtonSetManager", () => {
       const buttonSetWriter = createMockButtonSetWriter();
 
       const manager = ButtonSetManager.create(configManager, configReader, buttonSetWriter);
-      const result = await manager.updateButtonSet("set-1", { name: "SETA" });
+      const result = await manager.renameButtonSet("SetA", "SETA");
 
       expect(result.success).toBe(true);
       expect(buttonSetWriter.writeButtonSets).toHaveBeenCalled();

src/internal/managers/button-set-manager.ts

@@ -174,6 +174,41 @@ export class ButtonSetManager {
     return matchingSet?.buttons ?? null;
   }
 
+  async renameButtonSet(currentName: string, newName: string): Promise<ButtonSetOperationResult> {
+    const sets = this.getButtonSets();
+    const setIndex = sets.findIndex((set) => set.name.toLowerCase() === currentName.toLowerCase());
+
+    if (setIndex === -1) {
+      return { error: "setNotFound", success: false };
+    }
+
+    const validation = this.validateSetName(newName, currentName);
+    if (validation.error) {
+      return { error: validation.error, success: false };
+    }
+
+    const existingSet = sets[setIndex];
+    const updatedSet: ButtonSet = {
+      ...existingSet,
+      name: validation.trimmedName,
+    };
+
+    const updatedSets = [...sets];
+    updatedSets[setIndex] = updatedSet;
+
+    const currentTarget = this.configManager.getCurrentConfigurationTarget();
+    const activeSet = this.getActiveSet();
+    const needsActiveSetUpdate = activeSet?.toLowerCase() === currentName.toLowerCase();
+
+    await this.writeButtonSets(updatedSets, currentTarget);
+
+    if (needsActiveSetUpdate) {
+      await this.setActiveSet(validation.trimmedName);
+    }
+
+    return { success: true };
+  }
+
   async saveAsButtonSet(name: string): Promise<ButtonSetOperationResult> {
     const validation = this.validateSetName(name);
     if (validation.error) {
@@ -242,50 +277,6 @@ export class ButtonSetManager {
     return true;
   }
 
-  async updateButtonSet(
-    id: string,
-    updates: { buttons?: ButtonConfigWithOptionalId[]; name?: string }
-  ): Promise<ButtonSetOperationResult> {
-    const sets = this.getButtonSets();
-    const setIndex = sets.findIndex((set) => set.id === id);
-
-    if (setIndex === -1) {
-      return { error: "setNotFound", success: false };
-    }
-
-    const existingSet = sets[setIndex];
-
-    let validatedName: string | undefined;
-    if (updates.name !== undefined) {
-      const validation = this.validateSetName(updates.name, existingSet.name);
-      if (validation.error) {
-        return { error: validation.error, success: false };
-      }
-      validatedName = validation.trimmedName;
-    }
-
-    const updatedSet: ButtonSet = {
-      ...existingSet,
-      ...(validatedName !== undefined && { name: validatedName }),
-      ...(updates.buttons !== undefined && { buttons: ensureIdsInArray(updates.buttons) }),
-    };
-
-    const updatedSets = [...sets];
-    updatedSets[setIndex] = updatedSet;
-
-    const currentTarget = this.configManager.getCurrentConfigurationTarget();
-    const activeSet = this.getActiveSet();
-    const needsActiveSetUpdate = activeSet === existingSet.name && validatedName !== undefined;
-
-    await this.writeButtonSets(updatedSets, currentTarget);
-
-    if (needsActiveSetUpdate) {
-      await this.setActiveSet(validatedName!);
-    }
-
-    return { success: true };
-  }
-
   validateUniqueName(name: string): boolean {
     const sets = this.getButtonSets();
     return validateUniqueSetName(name, sets);

src/internal/providers/webview-provider.ts

@@ -377,22 +377,22 @@ export const handleWebviewMessage = async (
         });
         break;
       }
-      case MESSAGE_TYPE.UPDATE_BUTTON_SET: {
+      case MESSAGE_TYPE.RENAME_BUTTON_SET: {
         if (!buttonSetManager) {
           throw new Error(MESSAGES.ERROR.buttonSetManagerNotAvailable);
         }
-        const updateData = message.data as
-          | { buttons?: ButtonConfigWithOptionalId[]; id?: string; name?: string }
-          | undefined;
-        if (!updateData?.id) {
-          throw new Error("setIdRequired");
-        }
-        const updateResult = await buttonSetManager.updateButtonSet(updateData.id, {
-          buttons: updateData.buttons,
-          name: updateData.name,
+        const renameDataSchema = z.object({
+          currentName: z.string().min(1),
+          newName: z.string().min(1),
         });
-        if (!updateResult.success) {
-          throw new Error(updateResult.error || "Failed to update button set");
+        const parseResult = renameDataSchema.safeParse(message.data);
+        if (!parseResult.success) {
+          throw new Error(MESSAGES.ERROR.renameRequiresBothNames);
+        }
+        const { currentName, newName } = parseResult.data;
+        const renameResult = await buttonSetManager.renameButtonSet(currentName, newName);
+        if (!renameResult.success) {
+          throw new Error(renameResult.error || MESSAGES.ERROR.renameButtonSetFailed);
         }
         await vscode.commands.executeCommand(COMMANDS.REFRESH);
         webview.postMessage({

src/shared/constants.ts

@@ -27,12 +27,12 @@ export const MESSAGE_TYPE = {
   IMPORT_CONFIGURATION: "importConfiguration",
   IMPORT_PREVIEW_RESULT: "importPreviewResult",
   PREVIEW_IMPORT: "previewImport",
+  RENAME_BUTTON_SET: "renameButtonSet",
   SAVE_AS_BUTTON_SET: "saveAsButtonSet",
   SET_ACTIVE_SET: "setActiveSet",
   SET_CONFIG: "setConfig",
   SET_CONFIGURATION_TARGET: "setConfigurationTarget",
   THEME_CHANGED: "themeChanged",
-  UPDATE_BUTTON_SET: "updateButtonSet",
 } as const;
 
 export const MESSAGES = {
@@ -57,6 +57,8 @@ export const MESSAGES = {
       "Invalid import confirmation data. Please restart the import process from the preview dialog.",
     invalidSetConfigData: "Invalid data for setConfig: data is not an array.",
     noCommands: "No commands found",
+    renameButtonSetFailed: "Failed to rename button set",
+    renameRequiresBothNames: "Both current name and new name are required for rename",
     unknownError: "Unknown error occurred",
   },
   INFO: {

src/shared/types.ts

@@ -64,11 +64,11 @@ export type WebviewMessageType =
   | "getConfig"
   | "importConfiguration"
   | "previewImport"
+  | "renameButtonSet"
   | "saveAsButtonSet"
   | "setActiveSet"
   | "setConfig"
-  | "setConfigurationTarget"
-  | "updateButtonSet";
+  | "setConfigurationTarget";
 
 export type ExtensionMessageType =
   | "configData"
@@ -95,7 +95,7 @@ export type WebviewMessage = {
     | { setName?: string | null }
     | { strategy?: string; target?: string }
     | { buttons?: ButtonConfigWithOptionalId[]; name: string; sourceSetId?: string }
-    | { buttons: ButtonConfigWithOptionalId[]; id: string; name?: string };
+    | { currentName: string; newName: string };
   requestId?: string;
   target?: string;
   type: WebviewMessageType;

src/view/src/components/button-set-selector.tsx

@@ -1,4 +1,4 @@
-import { Check, Layers, Plus, Trash2 } from "lucide-react";
+import { Check, Layers, Pencil, Plus, Trash2 } from "lucide-react";
 import { useState } from "react";
 import { useTranslation } from "react-i18next";
 
@@ -16,6 +16,8 @@ import {
 
 import { CreateSetDialog } from "./create-set-dialog";
 import { DeleteSetDialog } from "./delete-set-dialog";
+import { RenameSetDialog } from "./rename-set-dialog";
+import type { ButtonSet } from "../../../shared/types";
 import { useVscodeCommand } from "../context/vscode-command-context";
 
 export const ButtonSetSelector = () => {
@@ -24,6 +26,7 @@ export const ButtonSetSelector = () => {
   const [isOpen, setIsOpen] = useState(false);
   const [showCreateDialog, setShowCreateDialog] = useState(false);
   const [showDeleteDialog, setShowDeleteDialog] = useState(false);
+  const [renameTarget, setRenameTarget] = useState<ButtonSet | null>(null);
 
   const displayName = activeSet ?? t("buttonSets.default");
 
@@ -32,6 +35,12 @@ export const ButtonSetSelector = () => {
     setIsOpen(false);
   };
 
+  const handleRenameClick = (e: React.MouseEvent, set: ButtonSet) => {
+    e.stopPropagation();
+    setIsOpen(false);
+    setRenameTarget(set);
+  };
+
   return (
     <>
       <DropdownMenu onOpenChange={setIsOpen} open={isOpen}>
@@ -73,6 +82,15 @@ export const ButtonSetSelector = () => {
               />
               <span className="flex-1 truncate">{set.name}</span>
               <span className="ml-2 text-xs text-muted-foreground">{set.buttons.length}</span>
+              <button
+                aria-label={t("buttonSets.renameSet")}
+                className="ml-2 p-1 rounded hover:bg-accent"
+                data-testid={`rename-set-${set.id}`}
+                onClick={(e) => handleRenameClick(e, set)}
+                type="button"
+              >
+                <Pencil aria-hidden="true" className="h-3 w-3" />
+              </button>
             </DropdownMenuItem>
           ))}
 
@@ -104,6 +122,11 @@ export const ButtonSetSelector = () => {
 
       <CreateSetDialog onOpenChange={setShowCreateDialog} open={showCreateDialog} />
       <DeleteSetDialog onOpenChange={setShowDeleteDialog} open={showDeleteDialog} />
+      <RenameSetDialog
+        onOpenChange={(open) => !open && setRenameTarget(null)}
+        open={renameTarget !== null}
+        targetSet={renameTarget}
+      />
     </>
   );
 };