docs: implementation plan for -l highlight + -R review loop

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

1 files changed, 504 insertions, 0 deletions

diff --git a/docs/superpowers/plans/2026-06-18-list-highlight-review.md b/docs/superpowers/plans/2026-06-18-list-highlight-review.md
new file mode 100644
index 0000000..ed07632
--- /dev/null
+++ b/docs/superpowers/plans/2026-06-18-list-highlight-review.md
@@ -0,0 +1,504 @@
+# `-l` highlight + `-R` review loop — Implementation Plan
+
+> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
+
+**Goal:** Highlight hint rows whose version equals the SBo `.info` version in `mkhint -l`, and add a `-R`/`--review` flag that walks each matched hint with a side-by-side diff and a `[K]eep/[D]elete/[S]kip` prompt.
+
+**Architecture:** Two independent flags (`SHOW_LIST`, `RUN_REVIEW`) replace the single `COMMAND="list"`. `list_hint_files` gains exact-match highlighting (TTY-only color, plus a test-only `MKHINT_FORCE_COLOR` knob) and collects matched packages into a global array `MATCHED_PKGS`. A new `review_hint_files` consumes that array. Removal is factored into `_remove_hint` shared with `delete_hint_file`.
+
+**Tech Stack:** bash, getopt, tput, git/diff, existing `tests/mkhint_test.sh` harness.
+
+---
+
+## File Structure
+
+- Modify `mkhint`:
+  - `show_help` — document `--review`/`-R`.
+  - `list_hint_files` — highlight matches, populate `MATCHED_PKGS`.
+  - `delete_hint_file` — extract removal into `_remove_hint`.
+  - new `review_hint_files`.
+  - `main` — flag parsing + dispatch for `-l`/`-R`/`-lR`.
+- Modify `mkhint.bash-completion` — add `--review -R` to `all_flags`.
+- Modify `tests/mkhint_test.sh` — add T36–T41.
+- Modify `CLAUDE.md` — docs.
+
+Note on existing test IDs: T32–T35 already exist (nvchecker quoting/path). New tests are **T36–T41**.
+
+---
+
+## Task 1: Extract `_remove_hint` helper
+
+**Files:**
+- Modify: `mkhint` (`delete_hint_file`, ~lines 590–613)
+
+- [ ] **Step 1: Add `_remove_hint` and call it from `delete_hint_file`**
+
+Replace the body of `delete_hint_file` (lines 590–613) with:
+
+```bash
+# Remove a hint file and its .bak if present. No existence guard, no exit —
+# safe to call inside loops. Echoes what was removed.
+_remove_hint() {
+    local full_path="$1"
+    rm "$full_path"
+    echo "Deleted: $full_path"
+    local bak_path="${full_path}.bak"
+    if [[ -f "$bak_path" ]]; then
+        rm "$bak_path"
+        echo "Deleted: $bak_path"
+    fi
+}
+
+delete_hint_file() {
+    local file="$1"
+    local normalized_file="${file}"
+
+    if [[ "$file" != *.hint ]]; then
+        normalized_file="${file}.hint"
+    fi
+
+    local full_path="${HINT_DIR}/${normalized_file}"
+
+    if [[ ! -f "$full_path" ]]; then
+        echo "Error: Hint file does not exist: $full_path" >&2
+        exit 2
+    fi
+
+    _remove_hint "$full_path"
+}
+```
+
+- [ ] **Step 2: Run existing delete tests to confirm no regression**
+
+Run: `bash tests/mkhint_test.sh 2>&1 | grep -E "T11|T12|hint deleted|bak deleted|delete missing"`
+Expected: those PASS lines present, no new FAIL.
+
+- [ ] **Step 3: Commit**
+
+```bash
+git add mkhint
+git commit -m "refactor: extract _remove_hint helper for reuse in review loop"
+```
+
+---
+
+## Task 2: Highlight matched rows in `list_hint_files`
+
+**Files:**
+- Modify: `mkhint` (`list_hint_files`, lines 82–119)
+- Test: `tests/mkhint_test.sh`
+
+- [ ] **Step 1: Write the failing test (T36)**
+
+Insert before the `# ─── SUMMARY` block (after T35, ~line 746):
+
+```bash
+# ── T36: -l highlights rows where hint version == SBo version ─────────────────
+echo ""
+echo "T36: -l highlights matching version rows, plain row for mismatch"
+rm -f "$MOCK_HINT"/*.hint "$MOCK_HINT"/*.bak 2>/dev/null
+# curl matches its .info (8.5.0); clion differs (hint 9.9.9 vs .info 2025.3)
+cat > "$MOCK_HINT/curl.hint" << 'EOF'
+VERSION="8.5.0"
+ARCH="x86_64"
+DOWNLOAD="https://curl.se/download/curl-8.5.0.tar.gz"
+MD5SUM="abc123def456abc123def456abc123de"
+EOF
+cat > "$MOCK_HINT/clion.hint" << 'EOF'
+VERSION="9.9.9"
+ARCH="x86_64"
+DOWNLOAD="UNSUPPORTED"
+MD5SUM=""
+EOF
+# force color so the highlight is observable through the $(...) pipe
+out=$(MKHINT_FORCE_COLOR=1 run_mkhint -l 2>&1)
+# matched curl row carries the color start code (ESC[33m); legend present
+echo "$out" | grep -q $'\033\[33m.*curl.hint' \
+    && { echo "  PASS: curl row highlighted"; (( PASS++ )); } \
+    || { echo "  FAIL: curl row not highlighted"; echo "$out" | cat -v | sed 's/^/        /'; (( FAIL++ )); ERRORS+=("T36 highlight"); }
+echo "$out" | grep -q "highlighted = hint version matches" \
+    && { echo "  PASS: legend shown"; (( PASS++ )); } \
+    || { echo "  FAIL: legend missing"; (( FAIL++ )); ERRORS+=("T36 legend"); }
+# clion row must NOT carry the color start code
+echo "$out" | grep "clion.hint" | grep -q $'\033\[33m' \
+    && { echo "  FAIL: clion wrongly highlighted"; (( FAIL++ )); ERRORS+=("T36 false hl"); } \
+    || { echo "  PASS: clion row plain"; (( PASS++ )); }
+```
+
+- [ ] **Step 2: Run it to verify it fails**
+
+Run: `bash tests/mkhint_test.sh 2>&1 | grep -A6 "^T36:"`
+Expected: FAIL on "curl row highlighted" / "legend shown" (no color/legend yet).
+
+- [ ] **Step 3: Implement highlighting in `list_hint_files`**
+
+Replace lines 82–119 (whole function) with:
+
+```bash
+list_hint_files() {
+    if [[ ! -d "$HINT_DIR" ]]; then
+        echo "Error: Hint directory does not exist: $HINT_DIR" >&2
+        exit 2
+    fi
+
+    # Color only on a TTY, or when forced for tests. tput optional.
+    local c_on="" c_off=""
+    if [[ -n "$MKHINT_FORCE_COLOR" || -t 1 ]]; then
+        if command -v tput &>/dev/null && tput setaf 3 &>/dev/null; then
+            c_on=$(tput setaf 3); c_off=$(tput sgr0)
+        else
+            c_on=$'\033[33m'; c_off=$'\033[0m'
+        fi
+    fi
+
+    echo "Hint files in: $HINT_DIR"
+    echo "======================================================="
+    printf "%-40s  %10s  %10s  %-20s  %s\n" "File" "HintVer" "SBOVer" "Category" "Created"
+    echo "-------------------------------------------------------"
+
+    MATCHED_PKGS=()
+    local count=0 matched=0
+    for file in "$HINT_DIR"/*.hint; do
+        if [[ -f "$file" ]]; then
+            local VER=$(grep "^VERSION" "$file" |cut -d '"' -f2)
+            local name=$(basename "$file")
+            local pkg="${name%.hint}"
+            local info_file
+            info_file=$(find "$REPO_DIR" -mindepth 2 -name "${pkg}.info" 2>/dev/null | head -1)
+            local SBO_VER=""
+            local category=""
+            if [[ -f "$info_file" ]]; then
+                SBO_VER=$(grep "^VERSION" "$info_file" | cut -d '"' -f2)
+                category=$(basename "$(dirname "$(dirname "$info_file")")")
+            fi
+            local date=$(stat -c "%y" "$file" | cut -d'.' -f1)
+            local row
+            row=$(printf "%-40s  %10s  %10s  %-20s  %s" "$name" "$VER" "$SBO_VER" "$category" "$date")
+            if [[ -n "$VER" && "$VER" == "$SBO_VER" ]]; then
+                printf "%s%s%s\n" "$c_on" "$row" "$c_off"
+                MATCHED_PKGS+=("$pkg")
+                matched=$((matched + 1))
+            else
+                printf "%s\n" "$row"
+            fi
+            count=$((count + 1))
+        fi
+    done
+
+    if [[ $count -eq 0 ]]; then
+        echo "  (no hint files found)"
+    fi
+
+    echo "======================================================="
+    echo "Total: $count file(s)"
+    if [[ $matched -gt 0 && -n "$c_on" ]]; then
+        echo "(highlighted = hint version matches SBo .info)"
+    fi
+}
+```
+
+- [ ] **Step 4: Run T36 to verify it passes**
+
+Run: `bash tests/mkhint_test.sh 2>&1 | grep -A6 "^T36:"`
+Expected: all T36 lines PASS.
+
+- [ ] **Step 5: Declare `MATCHED_PKGS` global near other state**
+
+Find the global declarations near the top (after the path config block) and add:
+
+```bash
+MATCHED_PKGS=()
+```
+
+(Search for an existing `DELETE_HINT_FILES=()` or similar array declaration and place it alongside. If none exists at top, the in-function reset is sufficient — skip this step.)
+
+- [ ] **Step 6: Commit**
+
+```bash
+git add mkhint tests/mkhint_test.sh
+git commit -m "feat: highlight hint rows whose version matches SBo .info in -l"
+```
+
+---
+
+## Task 3: Add `review_hint_files` and `-R`/`--review` flag
+
+**Files:**
+- Modify: `mkhint` (new function; `main` parsing/dispatch lines 752–849; `show_help`)
+- Test: `tests/mkhint_test.sh`
+
+- [ ] **Step 1: Add `review_hint_files` function**
+
+Insert immediately after `list_hint_files` (after its closing `}`):
+
+```bash
+# Show each matched hint side-by-side with its .info, prompt Keep/Delete/Skip.
+# Relies on MATCHED_PKGS populated by list_hint_files.
+review_hint_files() {
+    if [[ ${#MATCHED_PKGS[@]} -eq 0 ]]; then
+        echo "No hints match their SBo version; nothing to review."
+        return 0
+    fi
+
+    local deleted=0 kept=0
+    local pkg
+    for pkg in "${MATCHED_PKGS[@]}"; do
+        local hint="${HINT_DIR}/${pkg}.hint"
+        local info
+        info=$(find "$REPO_DIR" -mindepth 2 -name "${pkg}.info" 2>/dev/null | head -1)
+        [[ -f "$hint" ]] || continue
+
+        echo ""
+        echo "=== $pkg ==="
+        if command -v git &>/dev/null; then
+            git diff --no-index --color=auto "$hint" "$info" || true
+        else
+            diff -y --width="${COLUMNS:-160}" "$hint" "$info" || true
+        fi
+
+        local ans
+        read -r -p "Review $pkg: [K]eep / [D]elete / [S]kip (default Keep): " ans
+        case "$ans" in
+            [Dd])
+                _remove_hint "$hint"
+                deleted=$((deleted + 1))
+                ;;
+            [Ss]|[Kk]|"")
+                echo "Kept: $pkg"
+                kept=$((kept + 1))
+                ;;
+            *)
+                echo "Unrecognised answer; keeping $pkg"
+                kept=$((kept + 1))
+                ;;
+        esac
+    done
+
+    echo ""
+    echo "Reviewed ${#MATCHED_PKGS[@]} hint(s): deleted $deleted, kept $kept."
+}
+```
+
+- [ ] **Step 2: Add the failing tests (T37–T41)**
+
+Insert after T36 in `tests/mkhint_test.sh`:
+
+```bash
+# ── T37: -R delete a matched hint ─────────────────────────────────────────────
+echo ""
+echo "T37: -R answer D → matched hint and .bak removed"
+rm -f "$MOCK_HINT"/*.hint "$MOCK_HINT"/*.bak 2>/dev/null
+cat > "$MOCK_HINT/curl.hint" << 'EOF'
+VERSION="8.5.0"
+ARCH="x86_64"
+DOWNLOAD="https://curl.se/download/curl-8.5.0.tar.gz"
+MD5SUM="abc123def456abc123def456abc123de"
+EOF
+touch "$MOCK_HINT/curl.hint.bak"
+out=$(run_mkhint -R < <(printf 'D\n') 2>&1)
+assert_file_not_exists "curl hint deleted"        "$MOCK_HINT/curl.hint"
+assert_file_not_exists "curl bak deleted"         "$MOCK_HINT/curl.hint.bak"
+echo "$out" | grep -q "deleted 1" \
+    && { echo "  PASS: summary reports deleted 1"; (( PASS++ )); } \
+    || { echo "  FAIL: summary wrong"; echo "$out" | sed 's/^/        /'; (( FAIL++ )); ERRORS+=("T37 summary"); }
+
+# ── T38: -R keep a matched hint ───────────────────────────────────────────────
+echo ""
+echo "T38: -R answer K → matched hint unchanged"
+rm -f "$MOCK_HINT"/*.hint "$MOCK_HINT"/*.bak 2>/dev/null
+cat > "$MOCK_HINT/curl.hint" << 'EOF'
+VERSION="8.5.0"
+ARCH="x86_64"
+DOWNLOAD="https://curl.se/download/curl-8.5.0.tar.gz"
+MD5SUM="abc123def456abc123def456abc123de"
+EOF
+run_mkhint -R < <(printf 'K\n') >/dev/null 2>&1
+assert_file_exists "curl hint kept" "$MOCK_HINT/curl.hint"
+
+# ── T39: -R empty answer = keep ───────────────────────────────────────────────
+echo ""
+echo "T39: -R empty answer → kept (default)"
+run_mkhint -R < <(printf '\n') >/dev/null 2>&1
+assert_file_exists "curl hint kept on empty" "$MOCK_HINT/curl.hint"
+
+# ── T40: -R skip a matched hint ───────────────────────────────────────────────
+echo ""
+echo "T40: -R answer S → matched hint unchanged"
+run_mkhint -R < <(printf 'S\n') >/dev/null 2>&1
+assert_file_exists "curl hint kept on skip" "$MOCK_HINT/curl.hint"
+
+# ── T41: -R with no matches → nothing to review, exit 0 ───────────────────────
+echo ""
+echo "T41: -R no matched rows → nothing to review"
+rm -f "$MOCK_HINT"/*.hint "$MOCK_HINT"/*.bak 2>/dev/null
+# clion hint version differs from its .info → no match
+cat > "$MOCK_HINT/clion.hint" << 'EOF'
+VERSION="9.9.9"
+ARCH="x86_64"
+DOWNLOAD="UNSUPPORTED"
+MD5SUM=""
+EOF
+set +e
+out=$(run_mkhint -R < <(printf '\n') 2>&1)
+code=$?
+set -e
+assert_exit_code "no-match review exits 0" 0 "$code"
+echo "$out" | grep -q "nothing to review" \
+    && { echo "  PASS: nothing-to-review message"; (( PASS++ )); } \
+    || { echo "  FAIL: message missing"; echo "$out" | sed 's/^/        /'; (( FAIL++ )); ERRORS+=("T41 msg"); }
+```
+
+- [ ] **Step 3: Run tests to verify T37–T41 fail**
+
+Run: `bash tests/mkhint_test.sh 2>&1 | grep -A3 -E "^T3[789]:|^T4[01]:"`
+Expected: FAIL — `-R` not yet a known option (getopt error / unknown option).
+
+- [ ] **Step 4: Wire up `-R`/`--review` in `main`**
+
+In the getopt line (line 755), add `R` to short opts and `review` to long opts:
+
+```bash
+    parsed=$(getopt -o v:f:n:lcCdNhR \
+        --long version:,hintfile:,new:,list,clean,check,delete,no-dl,help,review \
+        -n 'mkhint' -- "$@") || { show_help; exit 1; }
+```
+
+Replace the `--list|-l` case (lines 774–777) with two independent toggles:
+
+```bash
+            --list|-l)
+                SHOW_LIST=1
+                shift
+                ;;
+            --review|-R)
+                RUN_REVIEW=1
+                shift
+                ;;
+```
+
+- [ ] **Step 5: Add dispatch for the list/review flags**
+
+In the `if [[ -z "$COMMAND" ]]` block (lines 816–825), the new flags are not `COMMAND`s — handle them after that block. Add, right before the `case "$COMMAND" in` (line 837):
+
+```bash
+    if [[ -n "$SHOW_LIST" || -n "$RUN_REVIEW" ]]; then
+        [[ -n "$SHOW_LIST" ]] && list_hint_files
+        if [[ -n "$RUN_REVIEW" ]]; then
+            # ensure MATCHED_PKGS is populated even when -l was not given
+            [[ -z "$SHOW_LIST" ]] && list_hint_files >/dev/null
+            review_hint_files
+        fi
+        exit $?
+    fi
+```
+
+Remove the now-dead `list)` case from the `case "$COMMAND"` block (lines 841–843), since `-l` no longer sets `COMMAND`. Leave the rest of the cases intact.
+
+- [ ] **Step 6: Initialize the flag globals**
+
+Near the top globals (where `NO_DL=0` / `DELETE_HINT_FILES=()` live), add:
+
+```bash
+SHOW_LIST=""
+RUN_REVIEW=""
+```
+
+- [ ] **Step 7: Run the new tests to verify they pass**
+
+Run: `bash tests/mkhint_test.sh 2>&1 | grep -A3 -E "^T3[6789]:|^T4[01]:"`
+Expected: all T36–T41 lines PASS.
+
+- [ ] **Step 8: Run the full suite**
+
+Run: `bash tests/mkhint_test.sh 2>&1 | tail -5`
+Expected: `Results: N passed, 0 failed`.
+
+- [ ] **Step 9: Document `--review` in `show_help`**
+
+Add to the Usage block (after the `--list` line, ~line 49):
+
+```
+  ./mkhint --review                             Review hints matching SBo version, keep/delete each
+```
+
+Add to the Options block (after `--list, -l`, ~line 59):
+
+```
+  --review, -R             Review hints whose version matches the SBo .info; diff + keep/delete
+```
+
+- [ ] **Step 10: Commit**
+
+```bash
+git add mkhint tests/mkhint_test.sh
+git commit -m "feat: add -R/--review loop with side-by-side diff and keep/delete prompt"
+```
+
+---
+
+## Task 4: Bash completion + CLAUDE.md docs
+
+**Files:**
+- Modify: `mkhint.bash-completion` (line 11)
+- Modify: `CLAUDE.md`
+
+- [ ] **Step 1: Add `--review -R` to completion flags**
+
+In `mkhint.bash-completion` line 11, append to `all_flags`:
+
+```bash
+    local all_flags="--version -v --hintfile -f --new -n --list -l --review -R --clean -c --check -C --delete -d --no-dl -N --help -h"
+```
+
+- [ ] **Step 2: Update CLAUDE.md — Running/Testing**
+
+Add under the run examples (after the `--list` group / near line 42):
+
+```bash
+bash mkhint --list                         # highlight hints whose version matches SBo .info
+bash mkhint --review                       # review matched hints: diff + [K]eep/[D]elete/[S]kip
+bash mkhint --list --review                # show highlighted table, then review
+```
+
+- [ ] **Step 3: Update CLAUDE.md — test table**
+
+Append rows to the coverage table:
+
+```
+| T36 | `-l` highlights rows where hint version == SBo version; mismatch plain |
+| T37 | `-R` answer D — matched hint and .bak removed, summary reports deleted 1 |
+| T38 | `-R` answer K — matched hint unchanged |
+| T39 | `-R` empty answer — kept (default) |
+| T40 | `-R` answer S — matched hint unchanged |
+| T41 | `-R` no matched rows — "nothing to review", exit 0 |
+```
+
+- [ ] **Step 4: Update CLAUDE.md — Key Behaviors**
+
+Add bullets:
+
+```
+- `--list` / `-l`: lists hints with `HintVer`/`SBOVer`; rows where the two are byte-equal are highlighted (color only on a TTY; plain when piped). Adds a legend when any row matched.
+- `--review` / `-R`: iterates only the matched (highlighted) hints. For each, shows the hint side-by-side with its `.info` (`git diff --no-index` if git present, else `diff -y`), then prompts `[K]eep / [D]elete / [S]kip` (default Keep). Delete removes the hint and its `.bak` via `_remove_hint`. Prints a deleted/kept summary. `-l` and `-R` combine: `-lR` shows the table first, then reviews. No matches → "nothing to review", exit 0.
+```
+
+- [ ] **Step 5: Run full suite one final time**
+
+Run: `bash tests/mkhint_test.sh 2>&1 | tail -3`
+Expected: `Results: N passed, 0 failed`.
+
+- [ ] **Step 6: Commit**
+
+```bash
+git add mkhint.bash-completion CLAUDE.md
+git commit -m "docs: document -l highlight and -R review; add to bash completion"
+```
+
+---
+
+## Self-Review Notes
+
+- **Spec coverage:** flag table (Task 3 dispatch), exact match (Task 2 `==`), TTY-only color + non-TTY plain (Task 2 `c_on` guard), legend (Task 2), git/diff fallback (Task 3), Keep/Delete/Skip + default + re-prompt (Task 3 `case`), `_remove_hint` refactor preserving `--delete` exit-2 (Task 1), no-match exit 0 (Task 3 / T41), all docs (Task 4). All covered.
+- **Type consistency:** `MATCHED_PKGS` populated in `list_hint_files`, consumed in `review_hint_files`; `SHOW_LIST`/`RUN_REVIEW` set in parsing, read in dispatch; `_remove_hint` defined Task 1, used Tasks 1 & 3.
+- **Test IDs:** T36–T41 (T32–T35 already taken by nvchecker tests).
+- **Note for executor:** `-t 1` is false under the `$(...)` test pipe, so T36 uses `MKHINT_FORCE_COLOR=1`. Real interactive use needs no env var.