takt: reviewコマンドにて、差分を確認するメニューを用意してほしい。その場でgitdiffをするような形かな。
This commit is contained in:
parent
19ced26d00
commit
120815e848
@ -2,14 +2,14 @@
|
|||||||
* Tests for review-tasks command
|
* Tests for review-tasks command
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import { describe, it, expect } from 'vitest';
|
import { describe, it, expect, vi } from 'vitest';
|
||||||
import {
|
import {
|
||||||
parseTaktWorktrees,
|
parseTaktWorktrees,
|
||||||
extractTaskSlug,
|
extractTaskSlug,
|
||||||
buildReviewItems,
|
buildReviewItems,
|
||||||
type WorktreeInfo,
|
type WorktreeInfo,
|
||||||
} from '../task/worktree.js';
|
} from '../task/worktree.js';
|
||||||
import { isBranchMerged, type ReviewAction } from '../commands/reviewTasks.js';
|
import { isBranchMerged, showFullDiff, type ReviewAction } from '../commands/reviewTasks.js';
|
||||||
|
|
||||||
describe('parseTaktWorktrees', () => {
|
describe('parseTaktWorktrees', () => {
|
||||||
it('should parse takt/ branches from porcelain output', () => {
|
it('should parse takt/ branches from porcelain output', () => {
|
||||||
@ -170,14 +170,35 @@ describe('buildReviewItems', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
describe('ReviewAction type', () => {
|
describe('ReviewAction type', () => {
|
||||||
it('should include try, merge, delete (no skip)', () => {
|
it('should include diff, try, merge, delete', () => {
|
||||||
const actions: ReviewAction[] = ['try', 'merge', 'delete'];
|
const actions: ReviewAction[] = ['diff', 'try', 'merge', 'delete'];
|
||||||
expect(actions).toHaveLength(3);
|
expect(actions).toHaveLength(4);
|
||||||
|
expect(actions).toContain('diff');
|
||||||
expect(actions).toContain('try');
|
expect(actions).toContain('try');
|
||||||
|
expect(actions).toContain('merge');
|
||||||
|
expect(actions).toContain('delete');
|
||||||
expect(actions).not.toContain('skip');
|
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', () => {
|
describe('isBranchMerged', () => {
|
||||||
it('should return false for non-existent project dir', () => {
|
it('should return false for non-existent project dir', () => {
|
||||||
// git merge-base will fail on non-existent dir
|
// git merge-base will fail on non-existent dir
|
||||||
|
|||||||
@ -5,7 +5,7 @@
|
|||||||
* try merge, merge & cleanup, or delete actions.
|
* 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 chalk from 'chalk';
|
||||||
import {
|
import {
|
||||||
removeWorktree,
|
removeWorktree,
|
||||||
@ -21,7 +21,7 @@ import { createLogger } from '../utils/debug.js';
|
|||||||
const log = createLogger('review-tasks');
|
const log = createLogger('review-tasks');
|
||||||
|
|
||||||
/** Actions available for a reviewed worktree */
|
/** Actions available for a reviewed worktree */
|
||||||
export type ReviewAction = 'try' | 'merge' | 'delete';
|
export type ReviewAction = 'diff' | 'try' | 'merge' | 'delete';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Check if a branch has already been merged into HEAD.
|
* Check if a branch has already been merged into HEAD.
|
||||||
@ -39,6 +39,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.
|
* Show diff stat for a branch and prompt for an action.
|
||||||
*/
|
*/
|
||||||
@ -66,6 +88,7 @@ async function showDiffAndPromptAction(
|
|||||||
const action = await selectOption<ReviewAction>(
|
const action = await selectOption<ReviewAction>(
|
||||||
`Action for ${item.info.branch}:`,
|
`Action for ${item.info.branch}:`,
|
||||||
[
|
[
|
||||||
|
{ label: 'View diff', value: 'diff', description: 'Show full diff in pager' },
|
||||||
{ label: 'Try merge', value: 'try', description: 'Merge without cleanup (keep worktree & branch)' },
|
{ 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' },
|
{ label: 'Merge & cleanup', value: 'merge', description: 'Merge (if needed) and remove worktree & branch' },
|
||||||
{ label: 'Delete', value: 'delete', description: 'Discard changes, remove worktree and branch' },
|
{ label: 'Delete', value: 'delete', description: 'Discard changes, remove worktree and branch' },
|
||||||
@ -214,7 +237,15 @@ export async function reviewTasks(cwd: string): Promise<void> {
|
|||||||
const item = items[selectedIdx];
|
const item = items[selectedIdx];
|
||||||
if (!item) continue;
|
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;
|
if (action === null) continue;
|
||||||
|
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user