diff --git a/tools/context-pack.py b/tools/context-pack.py index 13cbd96..7f9a80a 100644 --- a/tools/context-pack.py +++ b/tools/context-pack.py @@ -1,21 +1,26 @@ #!/usr/bin/env python3 """Carry a cut_spec's file_changes anchors from the spec's base to a worktree. -Usage: context-pack.py [--since ] [--only [,...]] +Usage: context-pack.py [--since ] [--only |r[,...]] [--spec-json ] [--max-lines ] -Prints a header, a status table (one line per anchor) and bounded windows of the -code each anchor names, numbered in the worktree's lines. Exit: 0 every anchor -carried; 3 an anchor cannot be carried (OUT_OF_RANGE, UNRESOLVED, GONE outside +Prints a header, a status table (one line per anchor: file_changes as , reads +as r) and bounded windows of the code each anchor names, numbered in the +worktree's lines: edit windows, then READ windows, then one caller list per +distinct symbol. A symbol an anchor names must be in its mapped window or the +status gains SYMBOL_MISSING. Exit: 0 every anchor carried; 3 an anchor cannot be +carried (OUT_OF_RANGE, UNRESOLVED, MISSING read, SYMBOL_MISSING, GONE outside the spec's deletes); 2 usage; 4 spec unreadable. Standard library plus git. Errors name a field and a code, never an input value. ---max-lines bounds the windows, not the table: the table always lists every -anchor, so a budget smaller than the table prints the table and no window. +--max-lines bounds the windows and caller lists, not the table: the table always +lists every anchor, so a budget smaller than the table prints the table and no +window. Precedence: table, edit windows, read windows, caller lists. """ import argparse import json import os import re +import shlex import subprocess import sys @@ -26,6 +31,12 @@ WINDOW_CAP = 100 CONTEXT = 3 DEFAULT_MAX_LINES = 800 HUNK = re.compile(r"^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@") +MAX_CALLER_SYMBOLS = 8 +MAX_CALLER_HITS = 50 +MAX_CALLER_FILES = 20 +HIT_TEXT = 160 +ONLY = re.compile(r"^(r?)([0-9]+)$") +IDENT = re.compile(r"^[A-Za-z0-9_]+$") HEX = re.compile(r"^[0-9a-fA-F]{7,40}$") @@ -84,6 +95,17 @@ def check_spec_shape(spec): raise Fail(4, "spec: %s: InvalidInput" % field) if not isinstance(spec["base"], str): bad("base") + def check_loc(at, loc): + if not isinstance(loc, dict): + bad(at) + if not isinstance(loc.get("path"), str): + bad(at + ".path") + if not is_int(loc.get("line")): + bad(at + ".line") + if loc.get("end_line") is not None and not is_int(loc["end_line"]): + bad(at + ".end_line") + if loc.get("symbol") is not None and not isinstance(loc["symbol"], str): + bad(at + ".symbol") fcs = spec["file_changes"] if not isinstance(fcs, list): bad("file_changes") @@ -91,17 +113,21 @@ def check_spec_shape(spec): at = "file_changes[%d]" % k if not isinstance(fc, dict): bad(at) - loc = fc.get("location") - if not isinstance(loc, dict): - bad(at + ".location") - if not isinstance(loc.get("path"), str): - bad(at + ".location.path") - if not is_int(loc.get("line")): - bad(at + ".location.line") - if loc.get("end_line") is not None and not is_int(loc["end_line"]): - bad(at + ".location.end_line") + check_loc(at + ".location", fc.get("location")) if "change" in fc and not isinstance(fc["change"], str): bad(at + ".change") + reads = spec.get("reads") + if reads is None: + reads = [] + if not isinstance(reads, list): + bad("reads") + for k, rd in enumerate(reads, 1): + at = "reads[%d]" % k + if not isinstance(rd, dict): + bad(at) + check_loc(at + ".location", rd.get("location")) + if "why" in rd and not isinstance(rd["why"], str): + bad(at + ".why") dels = spec.get("deletes", []) if not isinstance(dels, list): bad("deletes") @@ -236,12 +262,34 @@ def safe_path(path): and ".." not in path.replace("\\", "/").split("/") -def resolve(tree, k, fc, deleted_ok, base_ok, listed): +def has_symbol(symbol, lines): + text = "\n".join(lines) + if IDENT.match(symbol): + return re.search(r"(? %s:%d-%d" % a["now"] return line + " # " + " ".join(a["change"].split())[:70] @@ -294,7 +345,8 @@ def table_line(a): def window_body(tree, a): kind, path, s, e = a["window"] lines = tree.base_lines(path) if kind == "base" else tree.wt_lines(path) - cap = "== [%d] %s:%d-%d%s %s" % (a["k"], path, s, e, " AT BASE" if kind == "base" else "", a["status"]) + where = "%s:%d-%d%s %s" % (path, s, e, " AT BASE" if kind == "base" else "", a["status"]) + cap = ("== READ %s: %s (%s)" % (a["k"], a["change"][:200], where)) if a["read"] else "== [%s] %s" % (a["k"], where) if lines is None: return cap, ["(file not readable in the worktree)"] lo, hi = max(1, s - CONTEXT), min(len(lines), e + CONTEXT) @@ -318,6 +370,46 @@ def overlaps_since(tree, since, a): return False +def caller_list(wt, symbol, windows): + """The lines of one CALLERS list: hits of the symbol in the worktree outside + every mapped window of the spec.""" + args = ["grep", "-n", "-I", "--no-color", "-z", "-F"] + (["-w"] if IDENT.match(symbol) else []) + ["-e", symbol] + raw = git(wt, *args) or b"" + hits = [] + for rec in raw.decode("utf-8", "replace").split("\n"): + parts = rec.split("\0", 2) + if len(parts) < 3 or not parts[1].isdigit(): + continue + path, n = parts[0], int(parts[1]) + if any(path == wp and ws <= n <= we for wp, ws, we in windows): + continue + hits.append((path, n, parts[2].rstrip("\r"))) + counts = {} + for path, _, _ in hits: + counts[path] = counts.get(path, 0) + 1 + out = ["CALLERS %s: %d hits in %d files" % (symbol, len(hits), len(counts))] + if len(hits) <= MAX_CALLER_HITS: + out += ["%s:%d: %s" % (path, n, text[:HIT_TEXT]) for path, n, text in hits] + else: + ranked = sorted(counts.items(), key=lambda kv: (-kv[1], kv[0])) + out += ["%6d %s" % (c, path) for path, c in ranked[:MAX_CALLER_FILES]] + if len(ranked) > MAX_CALLER_FILES: + out.append("(%d more files)" % (len(ranked) - MAX_CALLER_FILES)) + out.append("narrow: git grep -n -I %s-F -e %s -- " % ("-w " if IDENT.match(symbol) else "", shlex.quote(symbol))) + return out + + +def parse_only(text, p): + """Labels: for file_changes, r for reads.""" + labels = set() + for x in text.split(","): + m = ONLY.match(x) + if not m: + p.error("") + labels.add(m.group(1) + str(int(m.group(2)))) + return labels + + def main(argv): p = Parser(prog="context-pack.py") p.add_argument("spec_id") @@ -329,19 +421,18 @@ def main(argv): ns = p.parse_args(argv) if ns.max_lines < 1: p.error("") - only = None - if ns.only is not None: - try: - only = {int(x) for x in ns.only.split(",")} - except ValueError: - p.error("") + only = parse_only(ns.only, p) if ns.only is not None else None wt = ns.worktree if git(wt, "rev-parse", "--git-dir") is None: raise Fail(2, "worktree: not a git worktree") spec = load_spec(ns.spec_id, ns.spec_json) fcs = spec["file_changes"] - if only is not None and not only <= set(range(1, len(fcs) + 1)): - raise Fail(2, "--only: index outside file_changes") + reads = spec.get("reads") or [] + if only is not None: + if not {x for x in only if x[0] != "r"} <= {str(i) for i in range(1, len(fcs) + 1)}: + raise Fail(2, "--only: index outside file_changes") + if not {x for x in only if x[0] == "r"} <= {"r%d" % i for i in range(1, len(reads) + 1)}: + raise Fail(2, "--only: index outside reads") base = str(spec["base"]) resolved = HEX.match(base) and git(wt, "rev-parse", "--verify", "--quiet", base + "^{commit}") tree = Tree(wt, base) @@ -352,7 +443,9 @@ def main(argv): git(wt, "rev-parse", "--verify", "--quiet", ns.since + "^{commit}") is None): raise Fail(2, "--since: not a commit in this worktree") deleted_ok = {d["path"] for d in spec.get("deletes", [])} - anchors = [resolve(tree, i + 1, fc, deleted_ok, bool(resolved), listed) for i, fc in enumerate(fcs)] + anchors = [resolve(tree, str(i + 1), fc, deleted_ok, bool(resolved), listed) for i, fc in enumerate(fcs)] + anchors += [resolve(tree, "r%d" % (i + 1), {"location": rd["location"], "change": rd.get("why", "")}, + deleted_ok, bool(resolved), listed, read=True) for i, rd in enumerate(reads)] head = (git(wt, "rev-parse", "--short", "HEAD") or b"none").decode().strip() out = ["context-pack %s base %s HEAD %s modified %d" % ( @@ -361,8 +454,8 @@ def main(argv): out.append("") want = [a for a in anchors if a["window"] and (only is None or a["k"] in only) and (ns.since is None or overlaps_since(tree, ns.since, a))] - table_len = len(fcs) - skipped = [] + table_len = len(anchors) + skipped, skipped_need = [], 0 left = ns.max_lines - len(out) for idx, a in enumerate(want): cap, body = window_body(tree, a) @@ -376,14 +469,36 @@ def main(argv): need = 1 + len(body) + 1 if need > left - reserve: skipped.append(a["k"]) + skipped_need += need continue out.append(cap) out += body if len(body) < full: - out.append("%d more lines: --only %d --max-lines %d" % (full - len(body), a["k"], table_len + full + 3)) + out.append("%d more lines: --only %s --max-lines %d" % (full - len(body), a["k"], table_len + full + 3)) left -= need if skipped: - out.append("not expanded: --only " + ",".join(map(str, skipped))) + out.append("not expanded: --only " + ",".join(skipped)) + left -= 1 + windows = [(a["window"][1], a["window"][2], a["window"][3]) for a in anchors + if a["window"] and a["window"][0] == "wt"] + symbols = list(dict.fromkeys(a["symbol"] for a in want if a["symbol"])) + lists = [(sym, caller_list(wt, sym, windows)) for sym in symbols[:MAX_CALLER_SYMBOLS]] + unlisted = symbols[MAX_CALLER_SYMBOLS:] + cut = [] + fits_all = sum(len(ls) for _, ls in lists) + (1 if unlisted else 0) <= left + for idx, (sym, ls) in enumerate(lists): + reserve = 0 if fits_all else 1 + if len(ls) > left - reserve: + cut.append((sym, len(ls))) + continue + out += ls + left -= len(ls) + if cut: + out.append("callers not printed: %s: --max-lines %d" % ( + ",".join(c[0] for c in cut), + len(out) - (1 if skipped else 0) + skipped_need + sum(c[1] for c in cut) + (1 if unlisted else 0))) + if unlisted: + out.append("callers not listed: " + ",".join(unlisted)) sys.stdout.reconfigure(encoding="utf-8", errors="replace", newline="\n") sys.stdout.write("\n".join(out) + "\n") return max([a["exit"] for a in anchors] or [0]) diff --git a/tools/fixtures/context-pack/spec-reads.json b/tools/fixtures/context-pack/spec-reads.json new file mode 100644 index 0000000..841e5d4 --- /dev/null +++ b/tools/fixtures/context-pack/spec-reads.json @@ -0,0 +1,17 @@ +{"id": {"id": "t:cut_spec:cut-x.r1", "kind": "CutSpec"}, + "status": "InForce", + "document": {"kind": "cut_spec", "value": { + "base": "BASE", "deletes": [], + "file_changes": [ + {"change": "symbol present in the moved window", "location": {"path": "a.txt", "line": 20, "end_line": 22, "symbol": "a21"}}, + {"change": "symbol the window lacks", "location": {"path": "b.txt", "line": 30, "end_line": 31, "symbol": "b99"}}, + {"change": "identifier with 3 callers outside the windows", "location": {"path": "sym.txt", "line": 4, "end_line": 6, "symbol": "needle_three"}}, + {"change": "non-identifier symbol is a literal substring", "location": {"path": "sym.txt", "line": 9, "end_line": 9, "symbol": "::ba"}} + ], + "reads": [ + {"why": "moved below the insertion", "location": {"path": "a.txt", "line": 20, "end_line": 22}}, + {"why": "deleted path", "location": {"path": "c.txt", "line": 2, "end_line": 3}}, + {"why": "path absent at base", "location": {"path": "sub/a.txt", "line": 1, "end_line": 2}}, + {"why": "identifier with 60 callers", "location": {"path": "sym.txt", "line": 7, "end_line": 8, "symbol": "needle_many"}}, + {"why": "an identifier matches whole words only", "location": {"path": "a.txt", "line": 20, "end_line": 22, "symbol": "a2"}} + ]}}} diff --git a/tools/test-tools.sh b/tools/test-tools.sh index 6360480..374a66f 100644 --- a/tools/test-tools.sh +++ b/tools/test-tools.sh @@ -236,6 +236,9 @@ seqf() { local i; for i in $(seq 1 "$2"); do echo "$1$i"; done; } g init -q seqf a 40 >"$R/a.txt"; seqf b 40 >"$R/b.txt"; seqf c 10 >"$R/c.txt"; seqf d 20 >"$R/d.txt" seqf e 10 >"$R/e.txt"; seqf f 20 >"$R/f.txt"; seqf g 300 >"$R/big.txt"; seqf h 5 | sed 's/$/ /' >"$R/h.txt"; seqf m 40 >"$R/m.txt" +printf 's1\ns2\ns3\ns4\ndef needle_three\ns6\ns7 needle_many\ns8\nfoo::bar\ns10\n' >"$R/sym.txt" +printf 'needle_three call 1\nneedle_three call 2\nneedle_three call 3\nneedle_threefold is another word\n' >"$R/callers.txt" +seqf "needle_many x." 30 >"$R/many1.txt"; seqf "needle_many y." 30 >"$R/many2.txt" g add -A; g commit -q -m base; BASE=$(git -C "$R" rev-parse HEAD) { sed -n 1,5p "$R/a.txt"; printf 'ins1\nins2\nins3\n'; sed -n '6,$p' "$R/a.txt"; } >"$R/a.new"; mv "$R/a.new" "$R/a.txt" sed -e '2,3d' -e 's/^b10$/b10-EDITED/' "$R/b.txt" >"$R/b.new"; mv "$R/b.new" "$R/b.txt" @@ -337,6 +340,54 @@ cin "window cut at 100 lines" "56 more lines: --only 1 --max-lines 160" r=$(cpo --spec-json "$S1" --only 1 --max-lines 160); cchk "--only with the suggested --max-lines" 0 "$r" cnot "full window needs no more-lines note" "more lines" [ "$(wc -l <"$T/cp.out" | tr -d ' ')" = 160 ] && echo "ok context-pack suggested --max-lines is exact" || { echo "FAIL context-pack suggested max: $(wc -l <"$T/cp.out")"; fail=1; } +# reads, symbols and caller lists +sed "s/\"BASE\"/\"$BASE\"/" "$CF/spec-reads.json" >"$T/cp-spec-reads.json"; SR=$(wp "$T/cp-spec-reads.json") +r=$(cpo --spec-json "$SR"); cchk "reads and symbols" 3 "$r" +cin "a read that moved" "[r1] a.txt:20-22 MOVED(+3) -> a.txt:23-25" +cin "a read on a deleted path is GONE" "[r2] c.txt:2-3 GONE (not named by the spec's deletes)" +cin "a read absent at base is MISSING" "[r3] sub/a.txt:1-2 MISSING (read path absent at base)" +cin "a present symbol adds no suffix" "[1] a.txt:20-22 MOVED(+3) -> a.txt:23-25" +cin "a missing symbol is named" "[2] b.txt:30-31 MOVED(-2) SYMBOL_MISSING b99" +cin "an identifier matches whole words only" "MOVED(+3) SYMBOL_MISSING a2 -> a.txt:23-25" +cin "a non-identifier symbol is a literal substring" "[4] sym.txt:9-9 SAME" +cnot "the substring symbol is not missing" "SYMBOL_MISSING ::ba" +cin "the read caption carries its why" "== READ r1: moved below the insertion (a.txt:23-25 MOVED(+3))" +cin "a GONE read prints its window at base" "== READ r2: deleted path (c.txt:2-3 AT BASE" +[ "$(grep -c '^\[' "$T/cp.out")" = 9 ] && echo "ok context-pack the table lists edits and reads" || { echo "FAIL context-pack table rows with reads"; fail=1; } +[ "$(grep -n '^== READ r1' "$T/cp.out" | cut -d: -f1)" -gt "$(grep -n '^== \[4\]' "$T/cp.out" | cut -d: -f1)" ] && echo "ok context-pack read windows follow the edit windows" || { echo "FAIL context-pack read window order"; fail=1; } +[ "$(grep -n '^CALLERS' "$T/cp.out" | head -1 | cut -d: -f1)" -gt "$(grep -n '^== READ r5' "$T/cp.out" | cut -d: -f1)" ] && echo "ok context-pack caller lists follow the read windows" || { echo "FAIL context-pack caller list order"; fail=1; } +cin "3 callers are all listed" "CALLERS needle_three: 3 hits in 1 files" +cin "caller hit format" "callers.txt:2: needle_three call 2" +cnot "the hit inside the symbol's own window is dropped" "sym.txt:5: def needle_three" +cnot "a longer word is no caller" "needle_threefold" +cin "60 hits print per-file counts" "CALLERS needle_many: 60 hits in 2 files" +cin "per-file count line" " 30 many1.txt" +cin "the narrowing command" "narrow: git grep -n -I -w -F -e needle_many -- " +cnot "60 hits print no hit lines" "many1.txt:1:" +r=$(cpo --spec-json "$SR" --only r1); cchk "--only r1 keeps the exit of all anchors" 3 "$r" +cin "--only r1 prints the read" "== READ r1: moved below the insertion" +cnot "--only r1 prints no other read" "== READ r2"; cnot "--only r1 prints no edit window" "== [1]" +r=$(cpo --spec-json "$SR" --only r9); cchk "--only past the reads" 2 "$r"; grep -qF "context-pack: --only: index outside reads" "$T/cp.err" && echo "ok context-pack names the reads bound" || { echo "FAIL context-pack reads bound"; fail=1; } +r=$(cpo --spec-json "$SR" --only 1,r4); cin "--only mixes edit and read labels" "CALLERS needle_many: 60 hits"; cnot "--only lists callers of the selected anchors only" "CALLERS b99" +r=$(cpo --spec-json "$SR" --since "$(git -C "$R" rev-parse HEAD)"); cnot "--since HEAD prints no read window" "== READ"; cnot "--since HEAD prints no caller list" "CALLERS" +r=$(cpo --spec-json "$SR" --max-lines 3); cchk "--max-lines below the table with reads" 3 "$r" +[ "$(grep -c '^\[' "$T/cp.out")" = 9 ] && echo "ok context-pack a small budget keeps every edit and read row" || { echo "FAIL context-pack small budget with reads"; fail=1; } +cnot "a small budget prints no window" "== "; cnot "a small budget prints no caller list" "CALLERS" +r=$(cpo --spec-json "$SR" --max-lines 60); cchk "caller lists cut by the budget" 3 "$r" +[ "$(wc -l <"$T/cp.out" | tr -d ' ')" -le 60 ] && echo "ok context-pack caller lists stay within --max-lines" || { echo "FAIL context-pack caller lists over budget"; fail=1; } +cin "the cut caller lists are named" "callers not printed: " +SUGG=$(grep -o '^callers not printed: .*--max-lines [0-9]*' "$T/cp.out" | grep -o '[0-9]*$') +r=$(cpo --spec-json "$SR" --max-lines "$SUGG"); cnot "the suggested --max-lines prints every caller list" "callers not printed" +[ "$(wc -l <"$T/cp.out" | tr -d ' ')" = "$SUGG" ] && echo "ok context-pack suggested caller --max-lines is exact" || { echo "FAIL context-pack suggested caller max: $(wc -l <"$T/cp.out") vs $SUGG"; fail=1; } +{ echo '{"base": "'$BASE'", "deletes": [], "file_changes": [' + for i in 1 2 3 4 5 6 7 8 9; do echo '{"change": "s'$i'", "location": {"path": "big.txt", "line": '$i', "end_line": '$i', "symbol": "g'$i'"}}'$([ $i -lt 9 ] && echo ,); done + echo ']}'; } >"$T/cp-spec9sym.json" +r=$(cpo --spec-json "$(wp "$T/cp-spec9sym.json")" --max-lines 400); cchk "nine symbols" 0 "$r" +[ "$(grep -c '^CALLERS' "$T/cp.out")" = 8 ] && echo "ok context-pack at most 8 caller lists" || { echo "FAIL context-pack caller list count"; fail=1; } +cin "the ninth symbol is named" "callers not listed: g9" +echo '{"base": "'$BASE'", "deletes": [], "file_changes": [], "reads": [{"why": "w", "location": {"path": "a.txt", "line": 20, "end_line": 22, "symbol": "a21"}}]}' >"$T/cp-spec-r0.json" +r=$(cpo --spec-json "$(wp "$T/cp-spec-r0.json")"); cchk "a spec of one read, symbol present" 0 "$r" +cin "a read with a symbol present: no suffix" "[r1] a.txt:20-22 MOVED(+3) -> a.txt:23-25 # w" # usage and unreadable spec; no error echoes an input value r=$(cpo --spec-json "$SP" --max-lines CANARY-usage); cchk "bad --max-lines" 2 "$r"; cnot "usage error echoes no value" "CANARY-usage" r=$(cpo --spec-json "$T/CANARY-nofile.json"); cchk "unreadable --spec-json" 4 "$r"; grep -qF "context-pack: --spec-json: unreadable; --spec-json reads a saved view" "$T/cp.err" && echo "ok context-pack names the unreadable --spec-json" || { echo "FAIL context-pack names the unreadable --spec-json"; fail=1; }; cnot "unreadable --spec-json echoes no path" "CANARY-nofile" @@ -446,6 +497,14 @@ cases = { "deletes-str": (lambda v: v.update(deletes="CANARY-del"), "deletes"), "deletes-nopath": (lambda v: v.update(deletes=[{"note": "CANARY-note"}]), "deletes[1].path"), "deletes-entry-str": (lambda v: v.update(deletes=["CANARY-d"]), "deletes[1].path"), + "reads-str": (lambda v: v.update(reads="CANARY-reads"), "reads"), + "read-entry-str": (lambda v: v.update(reads=["CANARY-read"]), "reads[1]"), + "read-nolocation": (lambda v: v.update(reads=[{"why": "w"}]), "reads[1].location"), + "read-path": (lambda v: v.update(reads=[{"why": "w", "location": {"path": ["CANARY-p"], "line": 1}}]), "reads[1].location.path"), + "read-line": (lambda v: v.update(reads=[{"why": "w", "location": {"path": "a.txt", "line": "CANARY-l"}}]), "reads[1].location.line"), + "read-why": (lambda v: v.update(reads=[{"why": {"CANARY": 1}, "location": {"path": "a.txt", "line": 1}}]), "reads[1].why"), + "read-symbol": (lambda v: v.update(reads=[{"why": "w", "location": {"path": "a.txt", "line": 1, "symbol": 7}}]), "reads[1].location.symbol"), + "edit-symbol": (lambda v: v["file_changes"][0]["location"].update(symbol=["CANARY-s"]), "file_changes[1].location.symbol"), } names = [] for name, (edit, field) in cases.items():