From 0ee0d7aacf0e6501ef5813280518ab8143daf089 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Thu, 18 Jun 2026 11:28:47 +0200 Subject: docs: implementation plan for -l highlight + -R review loop Co-Authored-By: Claude Opus 4.8 --- .../plans/2026-06-18-list-highlight-review.md | 504 +++++++++++++++++++++ 1 file changed, 504 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-18-list-highlight-review.md (limited to 'docs/superpowers/plans') 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. -- cgit v1.2.3