Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ review-codecommit lets you browse AWS CodeCommit repositories, view pull request
- Browse pull requests with status filter (Open / Closed / Merged, `f` key cycle)
- Search pull requests by title or author (`/` key)
- Paginate through PR lists (`n`/`p` keys)
- View PR details with color-coded unified diffs (green for additions, red for deletions)
- View PR details with color-coded unified diffs (green for additions, red for deletions) with line number gutter
- Read PR comments inline
- Post comments on pull requests (`c` key in PR detail view)
- Post inline comments on specific diff lines (`C` key at cursor position)
Expand All @@ -32,6 +32,7 @@ review-codecommit lets you browse AWS CodeCommit repositories, view pull request
- Close PRs without merging (`x` key)
- Commit-level review with Tab/Shift+Tab navigation between "All changes" and individual commits
- Cursor-based diff navigation with `>` marker
- Jump to next/previous change line with `]c`/`[c` (Vim diff-style)
- View PR activity timeline (created, approved, merged, status changes, etc.) (`A` key)
- Shell completion for bash, zsh, and fish (`--completions` option)
- Vim-style keybindings (`j`/`k` navigation)
Expand Down Expand Up @@ -107,6 +108,9 @@ Completions support:
| `Ctrl+d` | Half page down | PR Detail |
| `Ctrl+u` | Half page up | PR Detail |
| `G` | Jump to end | PR Detail |
| `n` / `N` | Next / previous file | PR Detail |
| `]c` | Jump to next change (add/delete line) | PR Detail |
| `[c` | Jump to previous change (add/delete line) | PR Detail |
| `Enter` | Select / confirm / submit comment | List screens / Comment input |
| `q` / `Esc` | Go back / cancel | All / Comment input / Confirm prompt |
| `c` | Post comment | PR Detail |
Expand Down
48 changes: 36 additions & 12 deletions docs/design-review-readability.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# レビュー画面の可読性改善設計書

> **ステータス**: 設計完了(2026-04-09
> **✅ 実装完了** (2026-04-09)
>
> PR詳細画面のdiff表示に行番号ガターと変更行ジャンプ(`]c`/`[c`)を追加し、コードレビュー体験を改善する。

Expand Down Expand Up @@ -60,10 +60,12 @@ PR詳細画面(`PullRequestDetail`)のdiff表示に2つの機能を追加し
`DiffLine.tsx` に `formatGutter(line: DisplayLine): string` ヘルパーを追加:
```typescript
function formatGutter(line: DisplayLine): string {
const before = line.beforeLineNumber ? String(line.beforeLineNumber).padStart(4) : " ";
const after = line.afterLineNumber ? String(line.afterLineNumber).padStart(4) : " ";
const before = line.beforeLineNumber !== undefined ? String(line.beforeLineNumber).padStart(4) : " ";
const after = line.afterLineNumber !== undefined ? String(line.afterLineNumber).padStart(4) : " ";
return `${before} ${after} │ `;
}
// Note: computeSimpleDiff() は bi+1/ai+1 で行番号を生成するため 0 は来ないが、
// DisplayLine の型定義(number | undefined)に対して防御的に !== undefined を使用する。
```

`add`, `delete`, `context` の各caseでガター部分を `<Text dimColor>` で先頭に追加:
Expand Down Expand Up @@ -121,6 +123,8 @@ Vim diff風の `]c` (次の変更行) / `[c` (前の変更行) キーバイン

`design-review-keybindings.md:349` で「2ストローク操作はタイムアウトベースのキーシーケンス管理が必要」と記載されている。しかしこれは `gg` のように**1キー目が既存キーバインドと衝突する場合**(`g` はリアクション操作に使用中)の話。`[`/`]` は現在どのキーバインドにも使用されていないため、押下時点で即座に `pendingBracket` state を設定する state ベースアプローチが使える。タイムアウト管理は不要。

**タイムアウトなしのUX影響**: `]` を押した後に長時間放置してから `c` を押すと、コメント投稿ではなく変更行ジャンプが発火する。ただし、間に別のキー(`j`/`k` 等)を1つでも押せば `pendingBracket` はリセットされるため、誤発火は「`]`→(何もしない)→`c`」のパターンに限定される。実用上問題にならないと判断し、タイムアウトは導入しない。

**state遷移図:**
```
[null] ──("]"キー)──→ ["]"] ──("c"キー)──→ [null] + 次の変更行へジャンプ
Expand Down Expand Up @@ -190,6 +194,8 @@ if (input === "[") {
| `[` 押下直後に `q` を押す | pendingBracket リセット、`q` で画面遷移 | 同上 |
| All changes / コミットビュー | 両方で動作する | diff行のtype判定は共通。`viewIndex` によるガードなし |
| 連続して `]c`/`[c` を押す | 変更行を順にたどれる | 毎回 `cursorIndex` から検索するため |
| `n`/`N` とのラップアラウンド差異 | `]c`/`[c` はラップアラウンドしない | `n`/`N`(ファイルジャンプ)はラップアラウンドするが、`]c`/`[c` はVimの `]c` に準拠しラップしない。意図的な設計差異 |
| コミットビューでの行番号ガター | 表示される | `DiffLine.tsx` はビューモード非依存の共通コンポーネントであり、コミットビューでも行番号ガターが表示される |

**キーバインドの整合性:**

Expand All @@ -201,6 +207,12 @@ if (input === "[") {
**フッター更新** (`PullRequestDetail.tsx` line 905付近):
`n/N file` の直後に `]c/[c change` を追加。

**`pendingBracket` の視覚的フィードバック**: `]` または `[` 押下時、フッターに pending 状態を表示する。`pendingBracket` が非 null の場合、通常のフッターヒントの代わりに以下を表示:
```
]_ Press c for next change, other key to cancel
```
これにより、ユーザーが2キー目の入力を待っている状態であることを明示する。Vimのpending operator表示に倣った設計。

**Help.tsx更新:**
Navigation セクション内(line 50-51 の `n`/`N` の直後)に追加:
```tsx
Expand Down Expand Up @@ -251,24 +263,25 @@ TDDサイクル:
- フッター情報量の削減(`design-review-keybindings.md` P2 として別途対応)
- 表示行数のターミナル高さ連動
- 要件定義書(`docs/requirements.md`)のキーバインド表への `]c`/`[c` 追記は実装完了後に `/update-docs` で対応
- ロードマップ(`docs/roadmap.md`)の v0.3.0 セクションへの行番号ガター・`]c`/`[c` 追記は実装完了後に `/update-docs` で対応

---

## update-plan 検証結果

### 設計書品質評価
### 設計書品質評価(レビュー反映後)

| # | カテゴリ | 点数 | コメント |
|---|---------|------|---------|
| 1 | 基本構造 | 9/10 | 概要・コンポーネント設計・実装詳細・テスト・検証方法・ファイル構成が揃っている |
| 2 | 目的・スコープの明確性 | 9/10 | 何を実装し何を実装しないかが明確。対象外も列挙済み |
| 3 | コンポーネント設計の完全性 | 9/10 | DiffLine.tsx の全case、pendingBracket の型定義state遷移図データフローを記載 |
| 3 | コンポーネント設計の完全性 | 9/10 | DiffLine.tsx の全case、pendingBracket の型定義state遷移図・視覚的フィードバック・データフローを記載 |
| 4 | AWS SDK連携設計 | N/A | 本変更にAWS SDK連携なし |
| 5 | 内部アーキテクチャ | 9/10 | ファイル構成表、データフロー図、state遷移図、`useInput` 内の配置順序を記載 |
| 6 | エッジケース・異常系 | 9/10 | 8ケースの網羅的な表、行番号オーバーフロー、モーダル中の挙動を記載 |
| 6 | エッジケース・異常系 | 9/10 | 10ケースの網羅的な表(`n`/`N` とのラップアラウンド差異、コミットビューのガター表示を追記)、行番号オーバーフロー、モーダル中の挙動を記載 |
| 7 | テスト戦略 | 9/10 | TDDサイクル(Red→Green→Refactor)で構成。具体的なテストケース、既存テスト影響分析を記載 |
| 8 | セキュリティ考慮 | N/A | 本変更にセキュリティ影響なし |
| 9 | 技術選定の根拠 | 9/10 | stateベース vs タイムアウトの比較、`design-review-keybindings.md` との整合性説明、ユーザー選択の経緯 |
| 9 | 技術選定の根拠 | 9/10 | stateベース vs タイムアウトの比較、タイムアウトなしのUX影響分析、`design-review-keybindings.md` との整合性説明を記載 |
| 10 | 実装計画 | 9/10 | 2イテレーション、Tidy First判定、TDDサイクル、対象ファイル/行番号が明確 |
| | **合計** | **72/80 (90%)** | N/A 2項目を除外した8項目での評価 |

Expand All @@ -278,14 +291,25 @@ TDDサイクル:

| チェック項目 | 結果 | 詳細 |
|-------------|------|------|
| 設計書 ↔ ソースコード | ✅ | `DisplayLine` に `beforeLineNumber`/`afterLineNumber` が既存(`formatDiff.ts:1-24`)。`DiffLine.tsx` の switch-case 構造と一致 |
| ロードマップ ↔ 設計書 ↔ 要件定義 | ✅ | v0.3.0 UX強化(diff可読性・ナビゲーション向上)に合致 |
| 設計書 ↔ ソースコード | ✅ | `DisplayLine` に `beforeLineNumber`/`afterLineNumber` が既存(`formatDiff.ts:1-24`)。`DiffLine.tsx` の switch-case 構造と一致。`formatGutter` は `!== undefined` で型安全にチェック |
| ロードマップ ↔ 設計書 ↔ 要件定義 | ✅ | v0.3.0 UX強化(diff可読性・ナビゲーション向上)に合致。`roadmap.md`・`requirements.md` の更新は対象外セクションで言及済み |
| CLAUDE.md との整合性 | ✅ | TDDサイクル、Tidy First判定、カバレッジ95%を担保 |
| ソースコード規約 | ✅ | 既存のキーハンドラパターン、`Help.tsx` の `SectionHeader`+`KeyRow` パターンに従う |
| design-review-keybindings.md | ✅ | 2ストローク操作にタイムアウト不要の根拠を記載 |
| design-review-keybindings.md | ✅ | 2ストローク操作にタイムアウト不要の根拠・UX影響を記載 |

### レビュー反映事項

以下の指摘をレビューに基づき設計書に反映済み:

- **P1-1**: `pendingBracket` の視覚的フィードバック → フッターに pending 状態表示を追加する設計を追記
- **P1-2**: `formatGutter` の truthiness チェック → `!== undefined` に変更し、`computeSimpleDiff` が1始まりである根拠をコメントで明記
- **P2-3**: `n`/`N` とのラップアラウンド差異 → エッジケース表に追記
- **P2-4**: タイムアウトなしのUX影響 → 2キーシーケンスの設計判断セクションに追記
- **P2-5**: コミットビューの行番号ガター → エッジケース表に追記
- **P2-6**: `roadmap.md` 更新 → 対象外セクションに追記

### 修正事項

- **P0**: なし
- **P1**: なし
- **P2**: フッターがさらに長くなる(`]c/[c change` 追加)が、`design-review-keybindings.md` P2(フッター整理)で別途対応
- **P1**: なし(反映済み)
- **P2**: フッターがさらに長くなる(`]c/[c change` + pending状態表示)が、`design-review-keybindings.md` P2(フッター整理)で別途対応
13 changes: 9 additions & 4 deletions docs/requirements.md
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,10 @@ npx review-codecommit --completions <shell> # シェル補完スクリプト生
| `Ctrl+d` | 半ページ下スクロール | PR詳細画面(コメント入力中・確認プロンプト中は無効) |
| `Ctrl+u` | 半ページ上スクロール | PR詳細画面(コメント入力中・確認プロンプト中は無効) |
| `G` | 最終行へジャンプ | PR詳細画面(コメント入力中・確認プロンプト中は無効) |
| `n` | 次のファイルへジャンプ | PR詳細画面(コメント入力中・確認プロンプト中は無効) |
| `N` | 前のファイルへジャンプ | PR詳細画面(コメント入力中・確認プロンプト中は無効) |
| `]c` | 次の変更行(add/delete)へジャンプ | PR詳細画面(コメント入力中・確認プロンプト中は無効) |
| `[c` | 前の変更行(add/delete)へジャンプ | PR詳細画面(コメント入力中・確認プロンプト中は無効) |
| `Enter` | 選択・決定 / コメント送信 | リスト画面 / コメント入力 |
| `q` / `Esc` | 前の画面に戻る / コメント入力キャンセル | 全画面 / コメント入力 |
| `Ctrl+C` | 即座に終了 | 全画面 |
Expand Down Expand Up @@ -258,12 +262,12 @@ npx review-codecommit --completions <shell> # シェル補完スクリプト生
│ src/auth.ts │
│ │
│ @@ -15,7 +15,7 @@ │
const config = {
│ > - timeout: 3000,
15 15 │ const config = {
│ > 16 │ - timeout: 3000,
│ 💬 taro: この値はconfigから取る方が良さそう │
│ └ watany: 次のPRで修正します │
+ timeout: 10000,
};
16 │ + timeout: 10000,
17 17 │ };
│ │
│──────────────────────────────────────────────│
│ Comments (3): │
Expand All @@ -282,6 +286,7 @@ npx review-codecommit --completions <shell> # シェル補完スクリプト生

- **形式**: unified diff (`git diff` 同等)
- **色付け**: 追加行=緑、削除行=赤、コンテキスト行=デフォルト
- **行番号ガター**: 変更前/変更後の行番号を各4桁右寄せで表示(例: ` 3 4 │ `)。add/delete/context行のみ。dimColorで本文色と分離
- **構造**: ファイル単位でセクション分け

## ページネーション
Expand Down
16 changes: 10 additions & 6 deletions docs/roadmap.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,17 +104,21 @@ PR レビューの完全なワークフロー(閲覧→コメント→承認

#### 機能

| 機能 | 内容 |
|------|------|
| シンタックスハイライト | diff 内のコードをファイル拡張子に応じて色付け表示 |
| ファイルツリー | PR 内の変更ファイルをツリー表示し、ファイル間をジャンプ(`t` キー) |
| diff 統計 | ファイル別・合計の追加行/削除行数を表示 |
| ファイルジャンプ | ファイルツリーで選択したファイルの diff 位置へ直接移動 |
| 機能 | 内容 | 状態 |
|------|------|------|
| 行番号ガター | diff の add/delete/context 行に変更前/変更後の行番号を表示(` 3 4 │ ` 形式) | ✅ |
| 変更行ジャンプ | `]c` で次の変更行、`[c` で前の変更行へジャンプ(Vim diff 風 2キーシーケンス) | ✅ |
| シンタックスハイライト | diff 内のコードをファイル拡張子に応じて色付け表示 | |
| ファイルツリー | PR 内の変更ファイルをツリー表示し、ファイル間をジャンプ(`t` キー) | |
| diff 統計 | ファイル別・合計の追加行/削除行数を表示 | |
| ファイルジャンプ | ファイルツリーで選択したファイルの diff 位置へ直接移動 | |

#### キーバインド追加

| キー | 動作 | 画面 |
|------|------|------|
| `]c` | 次の変更行(add/delete)へジャンプ | PR 詳細 |
| `[c` | 前の変更行(add/delete)へジャンプ | PR 詳細 |
| `t` | ファイルツリー表示 | PR 詳細 |

#### 設計上の考慮点
Expand Down
17 changes: 16 additions & 1 deletion src/components/DiffLine.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@ import { Text } from "ink";
import React from "react";
import type { DisplayLine } from "../utils/formatDiff.js";

function formatGutter(line: DisplayLine): string {
const before =
line.beforeLineNumber !== undefined ? String(line.beforeLineNumber).padStart(4) : " ";
const after =
line.afterLineNumber !== undefined ? String(line.afterLineNumber).padStart(4) : " ";
return `${before} ${after} │ `;
}

export function renderDiffLine(line: DisplayLine, isCursor = false): React.ReactNode {
const bold = isCursor;
switch (line.type) {
Expand All @@ -16,17 +24,24 @@ export function renderDiffLine(line: DisplayLine, isCursor = false): React.React
case "add":
return (
<Text color="green" bold={bold}>
<Text dimColor>{formatGutter(line)}</Text>
{line.text}
</Text>
);
case "delete":
return (
<Text color="red" bold={bold}>
<Text dimColor>{formatGutter(line)}</Text>
{line.text}
</Text>
);
case "context":
return <Text bold={bold}>{line.text}</Text>;
return (
<Text bold={bold}>
<Text dimColor>{formatGutter(line)}</Text>
{line.text}
</Text>
);
case "truncate-context":
return <Text dimColor>{line.text}</Text>;
case "truncation":
Expand Down
2 changes: 2 additions & 0 deletions src/components/Help.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ export function Help({ onClose }: Props) {
<KeyRow keyName="G" description="Jump to end" />
<KeyRow keyName="n" description="Next file" />
<KeyRow keyName="N" description="Previous file" />
<KeyRow keyName="]c" description="Next change (add/delete line)" />
<KeyRow keyName="[c" description="Previous change (add/delete line)" />
<KeyRow keyName="f" description="File list" />
<KeyRow keyName="Enter" description="Select / confirm" />
<KeyRow keyName="q / Esc" description="Back / quit" />
Expand Down
Loading
Loading