-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Removed hardcoded filenames to GlobalFileNames constants. #1904
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.
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
Merged
mrubens
merged 11 commits into
RooCodeInc:main
from
StevenTCramer:Cramer/2025-03-22/custom-modes-setting
Mar 24, 2025
Merged
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
f80a3c0
Replace hardcoded custom modes filename with GlobalFileNames constant
StevenTCramer 3b351b7
Remove 'cline' prefix from mcpSettings filename
StevenTCramer 151fabb
Use GlobalFileNames.customModes in modes.ts
StevenTCramer bbe91b4
replace two more `cline_mcp_settings` with `mcp_settings` in tests.
StevenTCramer 07a6220
feat: add settings file migration for new file naming convention
StevenTCramer 9aa112e
Add associated changeset
StevenTCramer 8cdd6d5
removed unused import
StevenTCramer 9aa617f
Merge branch 'RooVetGit:main' into Cramer/2025-03-22/custom-modes-set…
StevenTCramer df1ee64
Merge branch 'Cramer/2025-03-22/custom-modes-setting' of https://gith…
StevenTCramer 24cc9cf
refactor: move migrateSettings to dedicated utility file
StevenTCramer a2553e4
Update src/extension.ts
mrubens File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "roo-cline": patch | ||
| --- | ||
|
|
||
| Add settings migration to support renaming legacy settings files to new format |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| import * as vscode from "vscode" | ||
| import * as path from "path" | ||
| import * as fs from "fs/promises" | ||
| import { fileExistsAtPath } from "../utils/fs" | ||
| import { GlobalFileNames } from "../shared/globalFileNames" | ||
| import { migrateSettings } from "../utils/migrateSettings" | ||
|
|
||
| // Mock dependencies | ||
| jest.mock("vscode") | ||
| jest.mock("fs/promises") | ||
| jest.mock("fs") | ||
| jest.mock("../utils/fs") | ||
| // We're testing the real migrateSettings function | ||
|
|
||
| describe("Settings Migration", () => { | ||
| let mockContext: vscode.ExtensionContext | ||
| let mockOutputChannel: vscode.OutputChannel | ||
| const mockStoragePath = "/mock/storage" | ||
| const mockSettingsDir = path.join(mockStoragePath, "settings") | ||
|
|
||
| // Legacy file names | ||
| const legacyCustomModesPath = path.join(mockSettingsDir, "cline_custom_modes.json") | ||
| const legacyMcpSettingsPath = path.join(mockSettingsDir, "cline_mcp_settings.json") | ||
|
|
||
| // New file names | ||
| const newCustomModesPath = path.join(mockSettingsDir, GlobalFileNames.customModes) | ||
| const newMcpSettingsPath = path.join(mockSettingsDir, GlobalFileNames.mcpSettings) | ||
|
|
||
| beforeEach(() => { | ||
| jest.clearAllMocks() | ||
|
|
||
| // Mock output channel | ||
| mockOutputChannel = { | ||
| appendLine: jest.fn(), | ||
| append: jest.fn(), | ||
| clear: jest.fn(), | ||
| show: jest.fn(), | ||
| hide: jest.fn(), | ||
| dispose: jest.fn(), | ||
| } as unknown as vscode.OutputChannel | ||
|
|
||
| // Mock extension context | ||
| mockContext = { | ||
| globalStorageUri: { fsPath: mockStoragePath }, | ||
| } as unknown as vscode.ExtensionContext | ||
|
|
||
| // The fs/promises mock is already set up in src/__mocks__/fs/promises.ts | ||
| // We don't need to manually mock these methods | ||
|
|
||
| // Set global outputChannel for all tests | ||
| ;(global as any).outputChannel = mockOutputChannel | ||
| }) | ||
|
|
||
| it("should migrate custom modes file if old file exists and new file doesn't", async () => { | ||
| const mockCustomModesContent = '{"customModes":[{"slug":"test-mode"}]}' as string | ||
|
|
||
| // Mock file existence checks | ||
| ;(fileExistsAtPath as jest.Mock).mockImplementation(async (path: string) => { | ||
| if (path === mockSettingsDir) return true | ||
| if (path === legacyCustomModesPath) return true | ||
| if (path === newCustomModesPath) return false | ||
| return false | ||
| }) | ||
|
|
||
| await migrateSettings(mockContext, mockOutputChannel) | ||
|
|
||
| // Verify file was renamed | ||
| expect(fs.rename).toHaveBeenCalledWith(legacyCustomModesPath, newCustomModesPath) | ||
| }) | ||
|
|
||
| it("should migrate MCP settings file if old file exists and new file doesn't", async () => { | ||
| const mockMcpSettingsContent = '{"mcpServers":{"test-server":{}}}' as string | ||
|
|
||
| // Mock file existence checks | ||
| ;(fileExistsAtPath as jest.Mock).mockImplementation(async (path: string) => { | ||
| if (path === mockSettingsDir) return true | ||
| if (path === legacyMcpSettingsPath) return true | ||
| if (path === newMcpSettingsPath) return false | ||
| return false | ||
| }) | ||
|
|
||
| await migrateSettings(mockContext, mockOutputChannel) | ||
|
|
||
| // Verify file was renamed | ||
| expect(fs.rename).toHaveBeenCalledWith(legacyMcpSettingsPath, newMcpSettingsPath) | ||
| }) | ||
|
|
||
| it("should not migrate if new file already exists", async () => { | ||
| // Mock file existence checks | ||
| ;(fileExistsAtPath as jest.Mock).mockImplementation(async (path: string) => { | ||
| if (path === mockSettingsDir) return true | ||
| if (path === legacyCustomModesPath) return true | ||
| if (path === newCustomModesPath) return true | ||
| if (path === legacyMcpSettingsPath) return true | ||
| if (path === newMcpSettingsPath) return true | ||
| return false | ||
| }) | ||
|
|
||
| await migrateSettings(mockContext, mockOutputChannel) | ||
|
|
||
| // Verify no files were renamed | ||
| expect(fs.rename).not.toHaveBeenCalled() | ||
| }) | ||
|
|
||
| it("should handle errors gracefully", async () => { | ||
| // Mock file existence checks to throw an error | ||
| ;(fileExistsAtPath as jest.Mock).mockRejectedValue(new Error("Test error")) | ||
|
|
||
| // Set the global outputChannel for the test | ||
| ;(global as any).outputChannel = mockOutputChannel | ||
|
|
||
| await migrateSettings(mockContext, mockOutputChannel) | ||
|
|
||
| // Verify error was logged | ||
| expect(mockOutputChannel.appendLine).toHaveBeenCalledWith( | ||
| expect.stringContaining("Error migrating settings files"), | ||
| ) | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| import * as vscode from "vscode" | ||
| import * as path from "path" | ||
| import * as fs from "fs/promises" | ||
| import { fileExistsAtPath } from "./fs" | ||
| import { GlobalFileNames } from "../shared/globalFileNames" | ||
|
|
||
| /** | ||
| * Migrates old settings files to new file names | ||
| * | ||
| * TODO: Remove this migration code in September 2025 (6 months after implementation) | ||
| */ | ||
| export async function migrateSettings( | ||
| context: vscode.ExtensionContext, | ||
| outputChannel: vscode.OutputChannel, | ||
| ): Promise<void> { | ||
| // Legacy file names that need to be migrated to the new names in GlobalFileNames | ||
| const fileMigrations = [ | ||
| { oldName: "cline_custom_modes.json", newName: GlobalFileNames.customModes }, | ||
| { oldName: "cline_mcp_settings.json", newName: GlobalFileNames.mcpSettings }, | ||
| ] | ||
|
|
||
| try { | ||
| const settingsDir = path.join(context.globalStorageUri.fsPath, "settings") | ||
|
|
||
| // Check if settings directory exists first | ||
| if (!(await fileExistsAtPath(settingsDir))) { | ||
| outputChannel.appendLine("No settings directory found, no migrations necessary") | ||
| return | ||
| } | ||
|
|
||
| // Process each file migration | ||
| for (const migration of fileMigrations) { | ||
| const oldPath = path.join(settingsDir, migration.oldName) | ||
| const newPath = path.join(settingsDir, migration.newName) | ||
|
|
||
| // Only migrate if old file exists and new file doesn't exist yet | ||
| // This ensures we don't overwrite any existing new files | ||
| const oldFileExists = await fileExistsAtPath(oldPath) | ||
| const newFileExists = await fileExistsAtPath(newPath) | ||
|
|
||
| if (oldFileExists && !newFileExists) { | ||
| await fs.rename(oldPath, newPath) | ||
| outputChannel.appendLine(`Renamed ${migration.oldName} to ${migration.newName}`) | ||
| } else { | ||
| outputChannel.appendLine( | ||
| `Skipping migration of ${migration.oldName} to ${migration.newName}: ${oldFileExists ? "new file already exists" : "old file not found"}`, | ||
| ) | ||
| } | ||
| } | ||
| } catch (error) { | ||
| outputChannel.appendLine(`Error migrating settings files: ${error}`) | ||
| } | ||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.