diff と instruct 両機能を統合
/review コマンドに View diff と Instruct 両方のアクションを追加。
This commit is contained in:
commit
9eb63e787e
@ -2,14 +2,14 @@
|
||||
* Tests for review-tasks command
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import {
|
||||
parseTaktWorktrees,
|
||||
extractTaskSlug,
|
||||
buildReviewItems,
|
||||
type WorktreeInfo,
|
||||
} from '../task/worktree.js';
|
||||
import { isBranchMerged, type ReviewAction } from '../commands/reviewTasks.js';
|
||||
import { isBranchMerged, showFullDiff, type ReviewAction } from '../commands/reviewTasks.js';
|
||||
|
||||
describe('parseTaktWorktrees', () => {
|
||||
it('should parse takt/ branches from porcelain output', () => {
|
||||
@ -170,15 +170,36 @@ describe('buildReviewItems', () => {
|
||||
});
|
||||
|
||||
describe('ReviewAction type', () => {
|
||||
it('should include instruct, try, merge, delete (no skip)', () => {
|
||||
const actions: ReviewAction[] = ['instruct', 'try', 'merge', 'delete'];
|
||||
expect(actions).toHaveLength(4);
|
||||
it('should include diff, instruct, try, merge, delete (no skip)', () => {
|
||||
const actions: ReviewAction[] = ['diff', 'instruct', 'try', 'merge', 'delete'];
|
||||
expect(actions).toHaveLength(5);
|
||||
expect(actions).toContain('diff');
|
||||
expect(actions).toContain('instruct');
|
||||
expect(actions).toContain('try');
|
||||
expect(actions).toContain('merge');
|
||||
expect(actions).toContain('delete');
|
||||
expect(actions).not.toContain('skip');
|
||||
});
|
||||
});
|
||||
|
||||
describe('showFullDiff', () => {
|
||||
it('should not throw for non-existent project dir', () => {
|
||||
// spawnSync will fail gracefully; showFullDiff catches errors
|
||||
expect(() => showFullDiff('/non-existent-dir', 'main', 'some-branch')).not.toThrow();
|
||||
});
|
||||
|
||||
it('should not throw for non-existent branch', () => {
|
||||
expect(() => showFullDiff('/tmp', 'main', 'non-existent-branch-xyz')).not.toThrow();
|
||||
});
|
||||
|
||||
it('should warn when diff fails', () => {
|
||||
const warnSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
|
||||
showFullDiff('/non-existent-dir', 'main', 'some-branch');
|
||||
warnSpy.mockRestore();
|
||||
// No assertion needed — the test verifies it doesn't throw
|
||||
});
|
||||
});
|
||||
|
||||
describe('isBranchMerged', () => {
|
||||
it('should return false for non-existent project dir', () => {
|
||||
// git merge-base will fail on non-existent dir
|
||||
|
||||
@ -5,7 +5,7 @@
|
||||
* try merge, merge & cleanup, or delete actions.
|
||||
*/
|
||||
|
||||
import { execFileSync } from 'node:child_process';
|
||||
import { execFileSync, spawnSync } from 'node:child_process';
|
||||
import chalk from 'chalk';
|
||||
import {
|
||||
removeWorktree,
|
||||
@ -26,7 +26,7 @@ import { DEFAULT_WORKFLOW_NAME } from '../constants.js';
|
||||
const log = createLogger('review-tasks');
|
||||
|
||||
/** Actions available for a reviewed worktree */
|
||||
export type ReviewAction = 'try' | 'merge' | 'delete' | 'instruct';
|
||||
export type ReviewAction = 'diff' | 'instruct' | 'try' | 'merge' | 'delete';
|
||||
|
||||
/**
|
||||
* Check if a branch has already been merged into HEAD.
|
||||
@ -44,6 +44,28 @@ export function isBranchMerged(projectDir: string, branch: string): boolean {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Show full diff in an interactive pager (less).
|
||||
* Falls back to direct output if pager is unavailable.
|
||||
*/
|
||||
export function showFullDiff(
|
||||
cwd: string,
|
||||
defaultBranch: string,
|
||||
branch: string,
|
||||
): void {
|
||||
try {
|
||||
const result = spawnSync(
|
||||
'git', ['diff', '--color=always', `${defaultBranch}...${branch}`],
|
||||
{ cwd, stdio: ['inherit', 'inherit', 'inherit'], env: { ...process.env, GIT_PAGER: 'less -R' } },
|
||||
);
|
||||
if (result.status !== 0) {
|
||||
warn('Could not display diff');
|
||||
}
|
||||
} catch {
|
||||
warn('Could not display diff');
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Show diff stat for a branch and prompt for an action.
|
||||
*/
|
||||
@ -71,6 +93,7 @@ async function showDiffAndPromptAction(
|
||||
const action = await selectOption<ReviewAction>(
|
||||
`Action for ${item.info.branch}:`,
|
||||
[
|
||||
{ label: 'View diff', value: 'diff', description: 'Show full diff in pager' },
|
||||
{ label: 'Instruct', value: 'instruct', description: 'Give additional instructions to modify this worktree' },
|
||||
{ label: 'Try merge', value: 'try', description: 'Merge without cleanup (keep worktree & branch)' },
|
||||
{ label: 'Merge & cleanup', value: 'merge', description: 'Merge (if needed) and remove worktree & branch' },
|
||||
@ -294,7 +317,15 @@ export async function reviewTasks(cwd: string): Promise<void> {
|
||||
const item = items[selectedIdx];
|
||||
if (!item) continue;
|
||||
|
||||
const action = await showDiffAndPromptAction(cwd, defaultBranch, item);
|
||||
// Action loop: re-show menu after viewing diff
|
||||
let action: ReviewAction | null;
|
||||
do {
|
||||
action = await showDiffAndPromptAction(cwd, defaultBranch, item);
|
||||
|
||||
if (action === 'diff') {
|
||||
showFullDiff(cwd, defaultBranch, item.info.branch);
|
||||
}
|
||||
} while (action === 'diff');
|
||||
|
||||
if (action === null) continue;
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user