code review and small improvement

This commit is contained in:
Will Li 2025-07-21 12:43:43 -07:00
parent 415459910f
commit a7e4e5b7c6
6 changed files with 207 additions and 63 deletions

View file

@ -116,22 +116,6 @@ describe("ruleTypeDefinitions", () => {
expect(result).toContain("If found, incorporate and improve upon their content")
})
it("should include instructions to open files after generation", () => {
const ruleInstructions: RuleInstruction[] = [ruleTypeDefinitions.general]
const options: RulesGenerationOptions = {
selectedRuleTypes: ["general"],
addToGitignore: false,
alwaysAllowWriteProtected: false,
includeCustomRules: false,
customRulesText: "",
}
const result = generateRulesInstructions(ruleInstructions, options)
expect(result).toContain("Open the generated files")
expect(result).toContain("in the editor for review after creation")
})
it("should include proper formatting instructions", () => {
const ruleInstructions: RuleInstruction[] = [ruleTypeDefinitions.general]
const options: RulesGenerationOptions = {

View file

@ -49,11 +49,9 @@ ${ruleInstructions
5. **Keep rules concise** - aim for 20 lines per file, focusing on the most important guidelines
6. **Open the generated files** in the editor for review after creation
${
addToGitignore
? `7. **Add the generated files to .gitignore**:
? `6. **Add the generated files to .gitignore**:
- After generating all rule files, add entries to .gitignore to prevent them from being committed
- Add each generated file path to .gitignore (e.g., .roo/rules/coding-standards.md)
- If .gitignore doesn't exist, create it

View file

@ -1864,51 +1864,23 @@ export const webviewMessageHandler = async (
case "generateRules":
// Generate rules for the current workspace by spawning a new task
try {
const workspacePath = getWorkspacePath()
if (!workspacePath) {
vscode.window.showErrorMessage("No workspace folder open. Please open a folder to generate rules.")
break
}
// Import the rules generation service
const { createRulesGenerationTaskMessage } = await import("../../services/rules/rulesGenerator")
const { handleGenerateRules } = await import("../../services/rules/rulesGenerator")
// Get selected rule types and options from the message
const selectedRuleTypes = message.selectedRuleTypes || ["general"]
const addToGitignore = message.addToGitignore || false
const alwaysAllowWriteProtected = message.alwaysAllowWriteProtected || false
const apiConfigName = message.apiConfigName
const includeCustomRules = message.includeCustomRules || false
const customRulesText = message.customRulesText || ""
// Switch to the selected API config if provided
if (apiConfigName) {
const currentApiConfig = getGlobalState("currentApiConfigName")
if (apiConfigName !== currentApiConfig) {
await updateGlobalState("currentApiConfigName", apiConfigName)
await provider.activateProviderProfile({ name: apiConfigName })
}
}
// Create a comprehensive message for the rules generation task using existing analysis logic
const rulesGenerationMessage = await createRulesGenerationTaskMessage(
workspacePath,
selectedRuleTypes,
addToGitignore,
alwaysAllowWriteProtected,
includeCustomRules,
customRulesText,
// Call the refactored function with all necessary parameters
await handleGenerateRules(
provider,
{
selectedRuleTypes: message.selectedRuleTypes,
addToGitignore: message.addToGitignore,
alwaysAllowWriteProtected: message.alwaysAllowWriteProtected,
apiConfigName: message.apiConfigName,
includeCustomRules: message.includeCustomRules,
customRulesText: message.customRulesText,
},
getGlobalState,
updateGlobalState,
)
// Spawn a new task in code mode to generate the rules
await provider.initClineWithTask(rulesGenerationMessage)
// Automatically navigate to the chat tab to show the new task
await provider.postMessageToWebview({
type: "action",
action: "switchTab",
tab: "chat",
})
} catch (error) {
// Show error message to user
const errorMessage = error instanceof Error ? error.message : String(error)

View file

@ -1,13 +1,27 @@
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
import * as fs from "fs/promises"
import * as path from "path"
import { createRulesGenerationTaskMessage } from "../rulesGenerator"
import * as vscode from "vscode"
import { createRulesGenerationTaskMessage, handleGenerateRules } from "../rulesGenerator"
import { ClineProvider } from "../../../core/webview/ClineProvider"
// Mock fs module
vi.mock("fs/promises", () => ({
mkdir: vi.fn(),
}))
// Mock vscode module
vi.mock("vscode", () => ({
window: {
showErrorMessage: vi.fn(),
},
}))
// Mock getWorkspacePath
vi.mock("../../../utils/path", () => ({
getWorkspacePath: vi.fn(),
}))
describe("rulesGenerator", () => {
const mockWorkspacePath = "/test/workspace"
@ -178,4 +192,110 @@ describe("rulesGenerator", () => {
expect(message).toContain(".roo/rules-docs-extractor/documentation-rules.md")
})
})
describe("handleGenerateRules", () => {
let mockProvider: ClineProvider
let mockGetGlobalState: any
let mockUpdateGlobalState: any
let mockGetWorkspacePath: any
beforeEach(async () => {
// Mock provider
mockProvider = {
activateProviderProfile: vi.fn(),
initClineWithTask: vi.fn(),
postMessageToWebview: vi.fn(),
} as any
// Mock global state functions
mockGetGlobalState = vi.fn()
mockUpdateGlobalState = vi.fn()
// Import and mock getWorkspacePath
const pathModule = await import("../../../utils/path")
mockGetWorkspacePath = vi.spyOn(pathModule, "getWorkspacePath")
mockGetWorkspacePath.mockReturnValue(mockWorkspacePath)
})
it("should show error when no workspace is open", async () => {
mockGetWorkspacePath.mockReturnValue(undefined)
await handleGenerateRules(mockProvider, {}, mockGetGlobalState, mockUpdateGlobalState)
expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(
"No workspace folder open. Please open a folder to generate rules.",
)
expect(mockProvider.initClineWithTask).not.toHaveBeenCalled()
})
it("should switch API config when different from current", async () => {
mockGetGlobalState.mockReturnValue("current-config")
await handleGenerateRules(
mockProvider,
{ apiConfigName: "new-config" },
mockGetGlobalState,
mockUpdateGlobalState,
)
expect(mockUpdateGlobalState).toHaveBeenCalledWith("currentApiConfigName", "new-config")
expect(mockProvider.activateProviderProfile).toHaveBeenCalledWith({ name: "new-config" })
})
it("should not switch API config when same as current", async () => {
mockGetGlobalState.mockReturnValue("current-config")
await handleGenerateRules(
mockProvider,
{ apiConfigName: "current-config" },
mockGetGlobalState,
mockUpdateGlobalState,
)
expect(mockUpdateGlobalState).not.toHaveBeenCalled()
expect(mockProvider.activateProviderProfile).not.toHaveBeenCalled()
})
it("should create task and switch to chat tab", async () => {
await handleGenerateRules(
mockProvider,
{ selectedRuleTypes: ["general"] },
mockGetGlobalState,
mockUpdateGlobalState,
)
expect(mockProvider.initClineWithTask).toHaveBeenCalled()
expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({
type: "action",
action: "switchTab",
tab: "chat",
})
})
it("should pass all options to createRulesGenerationTaskMessage", async () => {
const options = {
selectedRuleTypes: ["general", "code"],
addToGitignore: true,
alwaysAllowWriteProtected: true,
includeCustomRules: true,
customRulesText: "Custom rules text",
}
await handleGenerateRules(mockProvider, options, mockGetGlobalState, mockUpdateGlobalState)
// Verify the task was created with the correct message
expect(mockProvider.initClineWithTask).toHaveBeenCalled()
const taskMessage = vi.mocked(mockProvider.initClineWithTask).mock.calls[0][0]
expect(taskMessage).toContain("Custom rules text")
})
it("should use default values when options are not provided", async () => {
await handleGenerateRules(mockProvider, {}, mockGetGlobalState, mockUpdateGlobalState)
expect(mockProvider.initClineWithTask).toHaveBeenCalled()
// The default selectedRuleTypes should be ["general"]
const taskMessage = vi.mocked(mockProvider.initClineWithTask).mock.calls[0][0]
expect(taskMessage).toContain(".roo/rules/coding-standards.md")
})
})
})

View file

@ -1,11 +1,15 @@
import * as fs from "fs/promises"
import * as path from "path"
import * as vscode from "vscode"
import { GlobalState } from "@roo-code/types"
import {
generateRulesInstructions,
ruleTypeDefinitions,
RulesGenerationOptions,
RuleInstruction,
} from "../../core/prompts/instructions/generate-rules"
import { getWorkspacePath } from "../../utils/path"
import { ClineProvider } from "../../core/webview/ClineProvider"
/**
* Creates a comprehensive task message for rules generation that can be used with initClineWithTask
@ -55,3 +59,69 @@ export async function createRulesGenerationTaskMessage(
return generateRulesInstructions(ruleInstructions, options)
}
/**
* Options for generating rules
*/
export interface GenerateRulesOptions {
selectedRuleTypes?: string[]
addToGitignore?: boolean
alwaysAllowWriteProtected?: boolean
apiConfigName?: string
includeCustomRules?: boolean
customRulesText?: string
}
/**
* Handles the complete rules generation process including API config switching,
* task creation, and UI navigation
*/
export async function handleGenerateRules(
provider: ClineProvider,
options: GenerateRulesOptions,
getGlobalState: <K extends keyof GlobalState>(key: K) => GlobalState[K],
updateGlobalState: <K extends keyof GlobalState>(key: K, value: GlobalState[K]) => Promise<void>,
): Promise<void> {
const workspacePath = getWorkspacePath()
if (!workspacePath) {
vscode.window.showErrorMessage("No workspace folder open. Please open a folder to generate rules.")
return
}
// Extract options with defaults
const selectedRuleTypes = options.selectedRuleTypes || ["general"]
const addToGitignore = options.addToGitignore || false
const alwaysAllowWriteProtected = options.alwaysAllowWriteProtected || false
const apiConfigName = options.apiConfigName
const includeCustomRules = options.includeCustomRules || false
const customRulesText = options.customRulesText || ""
// Switch to the selected API config if provided
if (apiConfigName) {
const currentApiConfig = getGlobalState("currentApiConfigName")
if (apiConfigName !== currentApiConfig) {
await updateGlobalState("currentApiConfigName", apiConfigName)
await provider.activateProviderProfile({ name: apiConfigName })
}
}
// Create a comprehensive message for the rules generation task
const rulesGenerationMessage = await createRulesGenerationTaskMessage(
workspacePath,
selectedRuleTypes,
addToGitignore,
alwaysAllowWriteProtected,
includeCustomRules,
customRulesText,
)
// Spawn a new task in code mode to generate the rules
await provider.initClineWithTask(rulesGenerationMessage)
// Automatically navigate to the chat tab to show the new task
await provider.postMessageToWebview({
type: "action",
action: "switchTab",
tab: "chat",
})
}

View file

@ -70,7 +70,7 @@ export const ExperimentalSettings = ({
})}
</Section>
<RulesSettings className="mt-6" hasUnsavedChanges={hasUnsavedChanges} />
<RulesSettings className="mt-3" hasUnsavedChanges={hasUnsavedChanges} />
</div>
)
}